MapColonies / error-express-handler

error middleware for @MapColonies/ts-server-boilerplate
0 stars 1 forks source link

Middleware won't catch errors in async routes #11

Open NivGreenstein opened 2 months ago

NivGreenstein commented 2 months ago

Describe the bug I've created custom Errors that implement HttpError interface from @map-colonies/error-express-handler. For non-async routes, when error is being thrown, the middleware catches it and sends the right error and http code. For async routes, when the same error is thrown, the server crash. In order to solve this issue, I had to add Try-Catch for every async route. With this patch, I once again got the server to respond with the expected output (expected http code and message).

To Reproduce Steps to reproduce the behavior:

  1. Create simple Express.JS server.
  2. Create custom error as described in the screenshots.
  3. Create routes as described in the screenshots.
  4. register getErrorHandlerMiddleware() from @map-colonies/error-express-handler as post routes middleware. Example: app.use(getErrorHandlerMiddleware());

Expected behavior I'd expect that the middleware will catch errors thrown at both async and non-async routes.

Screenshots The custom error implementation:

Screenshot 2024-09-11 at 18 12 53

Non-async route:

Screenshot 2024-09-11 at 18 13 25

Postman response (as expected):

Screenshot 2024-09-11 at 18 13 49

Async route that throws the same error:

Screenshot 2024-09-11 at 18 17 27

Error:

/<Path>/ts-server-boilerplate/src/serverBuilder.ts:72
        reject(new BadRequestError('temp route - bad request error'));
               ^

BadRequestError: temp route - bad request error
    at <anonymous> (/<Path>/ts-server-boilerplate/src/serverBuilder.ts:72:16)
    at new Promise (<anonymous>)
    at <anonymous> (/<Path>/ts-server-boilerplate/src/serverBuilder.ts:71:13)
    at <anonymous> (/<Path>/ts-server-boilerplate/node_modules/@opentelemetry/instrumentation-express/src/instrumentation.ts:223:27)
    at Layer.handle [as handle_request] (/<Path>/ts-server-boilerplate/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/<Path>/ts-server-boilerplate/node_modules/express/lib/router/index.js:328:13)
    at /<Path>/ts-server-boilerplate/node_modules/express/lib/router/index.js:286:9
    at Function.process_params (/<Path>/ts-server-boilerplate/node_modules/express/lib/router/index.js:346:12)
    at next (/<Path>/ts-server-boilerplate/node_modules/express/lib/router/index.js:280:10)
    at traceContexHeaderMiddleware (/<Path>/ts-server-boilerplate/node_modules/@map-colonies/telemetry/src/tracing/middleware/traceOnHeaderMiddleware.ts:16:12) {
  status: 400
}

Try-catch implementation:

Screenshot 2024-09-11 at 18 19 57
syncush commented 1 month ago

@netanelC @shimoncohen are you aware of this bug ?

syncush commented 1 month ago

@NivGreenstein please use reject, resolve and next correctly. You should

return reject(new BadRequestError("message"));

same for resolve and next

netanelC commented 1 month ago

@netanelC @shimoncohen are you aware of this bug ?

I wasn't aware, thanks