Fix slow RPC rejection on close; add browser integration tests #834
Conversation
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
Introduce a new React/browser integration test suite for AgentClient under packages/agents/src/react-tests. Adds comprehensive tests (connectivity, lifecycle, state sync, RPC, streaming, agentFetch, reconnection, query params, basePath routing, and multi-client scenarios). Add a global vitest setup that starts a Miniflare worker (setup.ts), test configuration helpers (test-config.ts), a tsconfig for the tests, and example useAgent tests. Update vitest config and agent test worker/wrangler settings to wire the suite up. Also add __screenshots__ to .gitignore to ignore Vitest browser screenshots.
Make AgentClient.close() immediately reject all pending RPC calls and clear pending state before closing the underlying socket. This provides immediate feedback on intentional disconnects (avoiding long WebSocket close-handshake timeouts). Also call stream.onError where applicable, add a changeset, and update the test to expect immediate rejection with the "Connection closed" error.
🦋 Changeset detectedLatest commit: 9a6fffd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as resolved.
This comment was marked as resolved.
commit: |
Extract repeated logic for rejecting and clearing pending RPC calls into a private _rejectPendingCalls(reason) method. Replace inline rejection code in the disconnect handler and close() to reduce duplication and ensure pending.stream?.onError is invoked with the provided reason (e.g., "Connection closed"). Adds a brief JSDoc for the new helper.
Add a note to AgentClient.close()'s documentation clarifying that any RPC calls made after close() will be rejected when the underlying WebSocket close event fires. This is a documentation-only change to make the rejection timing explicit and avoid confusion about calls made immediately after an intentional close.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
AgentClientanduseAgentReact hookAgentClient.close()to immediately reject pending RPC calls instead of waiting for WebSocket close handshake timeoutChanges
New Browser Integration Tests (~50 tests)
Tests run in a real browser (Playwright/Chromium) against a miniflare worker:
AgentClient tests:
agentFetchfor HTTP requestsuseAgent hook tests:
call()andstubproxyconsole.login theonMessagehandler ofuseAgentinagents/react#753)Bug Fix
AgentClient.close()now immediately rejects pending RPC calls. Previously, pending calls would wait for the WebSocket close handshake to complete (~15 seconds in some environments).Infrastructure
wrangler unstable_devtsconfig.jsonfor browser testsTest Plan
npm run check:test:react- 50 passed, 1 skippednpm run check:test:workers- 418 passed, 6 skipped