Fix WebSocket test flake and update UrlLib/AndroidExtensions#150
Merged
bghgary merged 9 commits intoBabylonJS:mainfrom Apr 13, 2026
Merged
Fix WebSocket test flake and update UrlLib/AndroidExtensions#150bghgary merged 9 commits intoBabylonJS:mainfrom
bghgary merged 9 commits intoBabylonJS:mainfrom
Conversation
When the WebSocket connection fails, onerror fires followed by onclose. Both called done(), causing mocha's 'done() called multiple times' error. Guard done() with a finished flag so only the first callback completes the test. This fixes the intermittent CI flake on Win32_x64_JSI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an intermittent Mocha CI flake in the Win32_x64_JSI WebSocket unit tests by preventing done() from being invoked multiple times when onerror is followed by onclose.
Changes:
- Added a
finishedflag andfinish()wrapper to ensure only the first callback completes the test. - Updated both the single-connection and multiple-connection WebSocket tests to use
finish()instead of callingdone()directly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bghgary
changed the title
Bump UrlLib to 880c2575e57ca0b59068ecc4860f185b9970e0ce (onClose after onError on all platforms) and AndroidExtensions to 2e85a8d43b89246c460112c9e5546ad54b6e87b4 (remove redundant errorCallback). With onclose guaranteed as the terminal event, tests now collect errors from earlier phases and report them in onclose — done() is called exactly once. The invalid domain test now verifies onerror fires before onclose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bghgary
changed the title
Replace expect() assertions with plain if-checks so async callbacks don't need try/catch. Each test calls done() exactly once from onclose (the terminal event). Errors from onerror or message validation are collected and passed through done(error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep expect() for all property and value checks. Errors are collected in an 'error' variable (typed unknown) and reported via done(error) in onclose. Catch blocks call ws.close() to ensure onclose fires promptly on assertion failure instead of waiting for timeout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ryantrem
approved these changes
Apr 13, 2026
bkaradzic-microsoft
approved these changes
Apr 13, 2026
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.
Fix WebSocket test flake and update dependencies.
The Bug
Tests called
done()from bothonerrorandonclose, causing mocha's "done() called multiple times" error when the native layer fired both events.Root Cause
The Windows and Apple WebSocket implementations only fired
onErroron connection failures, notonClose. Android'sorg.java_websocketlibrary fires both naturally. This inconsistency meant tests couldn't rely on a single terminal event.The Fix
UrlLib (bumped to 880c257): Added a single
CloseOncehelper on Windows andinvalidateAndCancelWithCloseCodeon Apple that checks the close code internally — firesonErrorfor abnormal codes, thenonCloseexactly once. All platforms now consistently fireonCloseas the terminal event.AndroidExtensions (bumped to 2e85a8d): Removed the redundant
errorCallbackfromonClosethat was re-interpreting close codes. Theorg.java_websocketlibrary already firesonErrorfor error conditions.Tests: Refactored so
done()is called exactly once fromonclose(the terminal event). Errors from earlier phases (assertion failures,onerror) are collected in anerrorvariable and reported viadone(error). Catch blocks callws.close()to ensureonclosefires promptly on assertion failure. Allexpectassertions are preserved for property and value checks (readyState,url, message content). The invalid domain test verifiesonerrorfires beforeonclose.Dependencies