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

fix: pass unhandled error on to next express error handler #1572

Merged

Conversation

luddwichr
Copy link
Contributor

Commit 0ae6f54 introduced an error handling middleware with this change. However, it does not delegate to the next error handler if it cannot handle the error adequately. This causes the request to hang until a timeout kills the connection. By passing the error on to next, other error handlers (like the default error handler) handle it.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

A request "hangs" may hang if an error is thrown in some express middleware.
In case ExpressLoader does not handle the error in its error handler function,
nothing happens with the request anymore and it hangs until the connection is terminated (e.g. due to timeout).

Issue Number: N/A (I didn't create a separate issue, let me know I you need that)

What is the new behavior?

the error handler defined in ExpressLoader calls next(err) in case it cannot adequately handle the error.
This way, other error handlers can take care of it.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Commit 0ae6f54 introduced an error handling middleware.
However, it does not delegate to the next error handler if it cannot handle the error adequately.
This causes the request to hang until a timeout kills the connection.
By passing the error on to next, other error handlers (like the default error handler) handle it.
@kamilmysliwiec kamilmysliwiec added the bug Something isn't working label Feb 6, 2025
@kamilmysliwiec kamilmysliwiec merged commit 6a67390 into nestjs:master Feb 6, 2025
1 check passed
@kamilmysliwiec
Copy link
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants