fix(sync,ready): distinguish MultiplayerTimeoutError from other wait() rejections - #45
Merged
Merged
Conversation
…) rejections Previously both plugins labeled every wait()/communicate() rejection as timed_out: true (a leftover from before #3694 exported a typed MultiplayerTimeoutError), which silently masked non-timeout failures (a throwing wait_for predicate, an adapter error) as ordinary timeouts. Now only a rejection named "MultiplayerTimeoutError" is recorded as a timeout; anything else propagates and fails the trial loudly, matching how a push() failure is already handled.
…ror typing plugin-multiplayer-role called the now-removed MultiplayerAPI.communicate() convenience method; it now calls push() followed by wait() directly. Also applies the same MultiplayerTimeoutError distinction already fixed in plugin-multiplayer-sync/ready (21e0909): only a rejection named "MultiplayerTimeoutError" is recorded as timed_out: true, so a throwing ready predicate or a push/adapter failure propagates and fails the trial loudly instead of being silently mislabeled a timeout. Docs cleanup: removed stale communicate() references from plugin-multiplayer-sync and adapter-multiplayer-jatos READMEs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
…ction The MultiplayerApiLike.wait() doc read as though a throwing `condition` also rejects with a MultiplayerTimeoutError; per jsPsych#3694 it rejects with the error the predicate threw (name != MultiplayerTimeoutError), which is exactly what the plugins' name-based guard depends on. Reword to say so plainly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on timeout path
On a genuine timeout the adapter may already be torn down, so api.getAll()
throws ("connect() must be called…"), which escaped and rejected the trial
instead of finishing it as timed_out. Wrap the snapshot read in the same
safeGetAll fallback plugin-multiplayer-ready already had, and mirror ready's
"getAll throws on the rejection path" regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the "MultiplayerTimeoutError" name match into a MULTIPLAYER_TIMEOUT_ERROR_NAME constant + isMultiplayerTimeoutError() type guard in each package's multiplayer-api.ts (sync/ready/role), and use it in the plugin guards and spec mocks. The literal previously appeared at six sites; an upstream rename in jsPsych#3694 is now a three-line change (one per package — the packages stay standalone by design). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Open
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
2 participants
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
MultiplayerTimeoutErrorfromwait()/communicate(), resolving a TODO both plugins carried since before that was finalized.plugin-multiplayer-syncandplugin-multiplayer-readypreviously treated everywait()rejection as a timeout — a throwingwait_forpredicate or an adapter/backend error would silently finish the trial withtimed_out: true, hiding the real failure inwait_error's message.error.name === "MultiplayerTimeoutError"is recorded as a timeout; anything else propagates and fails the trial loudly, matching how apush()failure is already handled in both plugins.Why this is its own PR
This changes trial data/behavior for a non-timeout failure (previously recorded as data, now throws), so it's a judgment call worth its own review rather than folding into the #3694-tracking SHA/
update()housekeeping in #41/#43/#44.Test plan
wait()to reject with aMultiplayerTimeoutError-named error, matching the real APInpx jestgreen for both packages (11/11 sync, 15/15 ready)tsc --noEmitclean for both packagesnpm run buildclean for both packageswait_errordescriptions and added a changeset