Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subsequent express middleware not invoked after OpenApiValidator #908

Open
dieseldjango opened this issue Mar 7, 2024 · 3 comments
Open

Comments

@dieseldjango
Copy link

Describe the bug
I have an express server that is configured with multiple middleware, including some before the openapi validator and some after, such as:

app.use(someMiddleware);
app.use(openApiValidator);
app.use(someOtherMiddleware);

When running the app the middleware registered after my OpenApiValidator instance is never getting called.
Is this to be expected? I was expecting that the validator would register express routes and would otherwise just call the next middleware in the chain.

To Reproduce
Setup as described below, write to console or set a breakpoint in someOtherMiddleware, start your server, send a request to one of the openapi endpoints.

Actual behavior
The someOtherMiddleware function is not called.

Expected behavior
The someOtherMiddleware function should be called.

Examples and context
Here's an example of what the config is like:

const openApiValidator = OpenApiValidator.middleware({
  apiSpec: OpenApiSpec,
  validateRequests: true,
  validateResponses: true,
  ignorePaths: (path: string) => !/^\/api/.test(path),
  operationHandlers: {
    basePath: "unused",
    resolver: (_basePath, route, apiDoc) => {
      const schema = apiDoc.paths[route.openApiRoute][route.method.toLowerCase()];
      const moduleName = schema["x-eov-operation-handler"];
      const functionName = schema["operationId"];
      const mod = openApiHandlers[moduleName];
      if (!mod) {
        throw new InternalServerError(
          `Could not find handler module "${moduleName}" for method "${route.method}" of route "${route.openApiRoute}"`
        );
      }
      const func = mod[functionName];
      if (func === undefined) {
        throw new Error(`Could not find function "${functionName}" in module "${moduleName}"`);
      }
      return func;
    },
  },
  validateSecurity: {
    handlers: {
      BearerAuth: openApiBearerTokenAuthHandler,
    },
  },
});
@JeffJassky
Copy link

JeffJassky commented Mar 11, 2024

I'm experiencing the same issue. It seems like it never attempts to resolve routes for subsequently defined middlewares. In my case, all of my spec files use the same root server path (https://server.com/api), and it happens regardless of how validateRequests, validateResponses, or operationHandlers are configured.

I have a hunch that a wildcard route for the API base url is created ( like /api/*) with a catch-all error handler at the end, that could essentially swallowing all subsequently defined routes to /api/*. I haven't looked at the source yet so this is just a hunch.

UPDATE:
I realized you described a completely different problem than I did. In my case I failed to read the docs! The main config includes an option called ignoreUndocumented that when enabled, will non-documented routes to pass through the middleware without throwing errors, allowing subsequent middleware to run. That way, I can stack multiple open api middlewares that handle different routes, and if it's not caught by the first one, it'll just try the next one.

@JeffJassky
Copy link

JeffJassky commented Mar 12, 2024

@dieseldjango if you need subsequent middleware to run, you need to make sure you return a valid express RequestHandler in the resolver method.

That means your openApiHandlers[moduleName][functionName] must return a function (req, res, next){...} and within that handler, you need to call the next() function.

For example, request handlers should operate like this:

openApiHandlers.moduleName.funcName = function handler(req, res, next){
  console.log('Handling the operation')
  next() <- this is what will forward the request to the next middleware
}

I assume that in the case of an InternalServerError, or in the case that func === undefined, you resolving to fail entirely. Alternatively, you could resolve a stub request handler that passes the error along to subsequent middlewares by returning a function(req, res, next){...}.

For example:

...
      if (!mod) {
        // return a valid request handler
        return (req, res, next) => {
          // Create your error without throwing it
          const error = new InternalServerError(
            `Could not find handler module "${moduleName}" for method "${route.method}" of route "${route.openApiRoute}"`
          )
          next(error) // call the next(), passing the error as the argument
        }
      }
      const func = mod[functionName];
      if (func === undefined) {
        // return a valid request handler
        return (req, res, next) => {
          // Create your error without throwing it
          const error = new Error(`Could not find function "${functionName}" in module "${moduleName}"`);
          next(error) // call the next(), passing the error as the argument
        }
    }
...

Now calls to invalid handlers will resolve successfully with a handler that passes an error along to the next middleware. This pattern or may not be ideal. I'd think that in most cases it would better for unresolved handlers to immediately fail catastrophically rather than silently passing an error along.

@dieseldjango
Copy link
Author

Thanks. I'll make sure to call next() in the route handler. However, in this case I want my middleware to run after the auth handler registered in validateSecurity but before the route handler. I can accomplish what I need by creating a wrapper for my route handlers, but I was hoping there'd be some way to register middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants