Repository navigation
fix: read abort reasons from captured signals - #1994
Merged
mcintyre94 merged 1 commit intoAug 26, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: fcfb7a5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
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 |
Member
trevor-cortex
left a comment
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Works around a React Native 0.86 quirk where the abort event delivered to a listener can have target === null, which made (e.target as AbortSignal).reason throw. Both call sites now read .reason from the signal already captured in scope (signal in the RPC request coalescer, callerAbortSignal in the timeout confirmation strategy), and regression tests simulate the null-target event in both packages.
Why this is correct
In both cases the listener is registered directly on the same signal object that's now being read. Per spec, e.target for an abort event is that signal, so signal.reason is semantically identical to the old expression in conforming environments — and it keeps working where the event implementation fails to set target. Dropping the now-unused e parameter is the right cleanup, and the as AbortSignal casts go away as a bonus.
The tests take the only realistic route to reproduce this in Node: spy out addEventListener, capture the handler, and invoke it manually with a synthetic Event('abort') whose target is asserted to be null before the handler runs. Aborting the controller first ensures signal.reason is populated, mirroring the real-world sequence. Slightly implementation-coupled, but appropriate here.
Changeset with patch bumps for @solana/rpc and @solana/transaction-confirmation matches the scope of the fix.
Notes for other reviewers
- It may be worth a quick repo-wide grep for other
abortlisteners readinge.target(e.g. in the subscriptions or WebSocket packages) so the same RN issue doesn't resurface elsewhere. This PR fixes the two sites named in the linked issue. - No behavioral change for platforms with conforming
AbortSignalimplementations — this is purely a defensive source change.
LGTM — would approve, but I don't have approval rights on this repo.
BundleMonUnchanged files (150)
Total files change -40B -0.01% Final result: ✅ View report in BundleMon website ➡️ |
Contributor
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Problem
In React Native 0.86, the abort event passed to the RPC coalescer can have
target === null. Readingevent.target.reasonthen throws:TypeError: Cannot read property 'reason' of nullI ran into this while testing Solana transaction confirmation in Trezor Suite: trezor/trezor-suite#30691
The timeout confirmation code used the same pattern.
Summary of Changes
Use the abort signal already in scope instead of reading it from the event. Added regression tests for both places.
I ran the node and browser tests, typecheck, lint, and Prettier for both affected packages.