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(WebSocket): support passthrough, correctly emit the error event #608

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Jul 31, 2024

Warning

Since WebSocket connections will now be established as-is if you haven't provided any interceptors, this is technically a breaking change despite fixing a bug.

Interceptors has a bug that it will always emit the open even on the WebSocket client, even when there are no interceptor set up for any connections (so the connection cannot physically be intercepted). This results in such connections pending forever.

This may be a big against previously established expectations of WebSocket interception always being mock-first. But this is still a bug for bypassed connections, and it has to be fixed.

I'm fixing this here without breaking any previously established promises.

  • WebSocket connection will be mock-first if at least one connection listener on the interceptor is present;
  • WebSocket connection will be established as-is if no connection listeners on the interceptors are present (previously, the connection would be OPEN and pend forever).

Discoveries

Roadmap

  • Add a failing test.
  • Establish the connection as-is if there were no interceptors set (this.emit() returned false).
  • Fix the issue where .close() would dispatch an error event on non-configurable closure codes (that's not how WebSocket behave).

@kettanaito
Copy link
Member Author

Alternative solution

Push the WebSocket interception a layer higher, letting the interceptor decide on the connection, whether to mock it, error it, or establish as-is.

interceptor.on('connection', ({ handshake }) => {
  handshake.accept()
  // handleshake.errorWith()
  // handshake.passthrough()
})

This may also be an answer to handling the handshake (protocol upgrade) request. It's been reported that we don't do that at the moment, which may cause issues (mostly in the browser).

@kettanaito kettanaito marked this pull request as ready for review August 1, 2024 10:33
@kettanaito kettanaito changed the title fix(WebSocket): emit "error" event on bypass connections feat(WebSocket): emit "error" event on bypass connections Aug 1, 2024
@kettanaito kettanaito changed the title feat(WebSocket): emit "error" event on bypass connections feat(WebSocket): support passthrough, correctly emit the error event Aug 1, 2024
@kettanaito kettanaito merged commit 5d9d71e into main Aug 1, 2024
2 checks passed
@kettanaito kettanaito deleted the fix/ws-client-error branch August 1, 2024 10:45
@kettanaito
Copy link
Member Author

Released: v0.34.0 🎉

This has been released in v0.34.0!

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


Predictable release automation by @ossjs/release.

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.

1 participant