fix(browser): surface mqtt-js errors during CONNECT as CONNECTION_FAILURE#698
Open
kamal wants to merge 2 commits into
Open
fix(browser): surface mqtt-js errors during CONNECT as CONNECTION_FAILURE#698kamal wants to merge 2 commits into
kamal wants to merge 2 commits into
Conversation
…LURE When the browser MQTT5 client encounters an error from mqtt-js during the Connecting lifecycle state (e.g. mqtt-js rejecting a Uint8Array password with "Invalid password"), the error is stored in lastError and emitted on the INFO event, but no CONNECTION_FAILURE is ever emitted. The underlying WebSocket can remain open indefinitely with no MQTT frames sent, causing the connection to hang silently. Force-close the mqtt-js client when an error occurs during Connecting so that the on_browser_close path fires and emits CONNECTION_FAILURE as expected. This allows consumers and the reconnection scheduler to react to the failure instead of hanging forever. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a test that verifies connectionFailure is emitted when mqtt-js rejects the password during CONNECT packet serialisation (e.g. when a Uint8Array is passed instead of string|Buffer in the browser). Without the fix in on_browser_client_error, this test would hang forever because the error was only emitted on the INFO event and the WebSocket would never close. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
This seems like a reasonable workaround (tbh I think mqtt-js should handle this case on their own as well). However, we are very near moving off of mqtt-js entirely (probably a couple of weeks) so I'm unsure if it's worth pushing this through. If you feel strongly about this I can push it through in the short-term though. |
Author
|
@bretambrose I don't feel strongly about it. Interested to know what you'll be moving to? |
Contributor
An internal implementation that follows the same architecture and approach as the native mqtt5 client. PRs are up; what remains is an integration PR that switches everything over and some stress testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the browser MQTT5 client encounters an error from mqtt-js during the
Connectinglifecycle state, the error is silently swallowed:on_browser_client_errorstores the error inlastErrorand emits it on theINFOeventCONNECTION_FAILUREevent is ever emittedReproduction
One concrete way to trigger this: pass a
Uint8Arrayas the password tonewWebsocketMqttBuilderWithCustomAuth. The CRT types accept binary data, but the underlying mqtt-js v4writeToStreamrejects anything that isn'tstring | BufferwithError: Invalid password. In the browser there is no nativeBuffer, soUint8Array instanceof Bufferis alwaysfalse.The WebSocket handshake succeeds (HTTP 101), but mqtt-js fails to serialize the CONNECT packet and emits an
errorevent. The CRT wrapper catches this inon_browser_client_error, stores it inlastError, and emits onINFO— but since no MQTT frames are sent, the server never closes the WebSocket,on_browser_closeis never called, andCONNECTION_FAILUREis never emitted.Fix
When an error occurs while
lifecycleEventState == Connecting, force-close the mqtt-js client so that the normalon_browser_close→CONNECTION_FAILUREpath fires. This lets consumers and the reconnection scheduler react to the failure.Test plan
CONNECTION_FAILUREinstead of hanging🤖 Generated with Claude Code