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

feat: treat unhandled exceptions in handlers as 500 error responses #2135

Merged
merged 10 commits into from
May 8, 2024

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Apr 17, 2024

Todo

  • Decide how to release this. It is technically a breaking change. Requests will not error where they used to error before. Tests may fail, angry comments may rise.

@kettanaito
Copy link
Member Author

Need to adjust some tests to reflect the Interceptors change.

@kettanaito kettanaito self-assigned this Apr 18, 2024
@kettanaito
Copy link
Member Author

The problem was that we throw an error in onUnhandledRequest and we intend that error to be forwarded to the process but since it happens within the request listener, it gets treated as an unhandled error and translated to the 500 error response. This hurts observability as it masks the error as an error response.

I've merged mswjs/interceptors#566 to allow us to opt-out from that default behavior and handle our internal errors differently (e.g. by forwarding them to the process).

@@ -29,3 +29,7 @@ export const devUtils = {
warn,
error,
}

export class InternalError extends Error {
//
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay now, this is embarrassing.

* forwarding the actual rejection in the "error.cause" property.
*/
expect(requestError.cause).toEqual(
expect(requestError).toEqual(
new Error(
'[MSW] Cannot bypass a request when using the "error" strategy for the "onUnhandledRequest" option.',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a correct "breaking" change. Errors originating from MSW must not be treated as request errors.

@kettanaito kettanaito merged commit 5191399 into main May 8, 2024
12 checks passed
@kettanaito kettanaito deleted the fix/interceptors-node-condition branch May 8, 2024 13:34
@kettanaito
Copy link
Member Author

Released: v2.3.0 🎉

This has been released in v2.3.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@jrnail23
Copy link

jrnail23 commented May 9, 2024

While I don't love the breaking change with a minor rev, this is a good change!
It actually allowed me to undo a bunch of special casing I introduced into my tests to handle the previously unhandled errors as I'm porting my http stubbing library from nock to msw.

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

Successfully merging this pull request may close these issues.

2 participants