Skip to content

fix: avoid unhandled rejection when DeferrableMap.getDeferred is aborted - #2156

Open
Abhishek-B-R wants to merge 1 commit into
livekit:mainfrom
Abhishek-B-R:fix/deferrable-map-abort-unhandled-rejection
Open

Abhishek-B-R wants to merge 1 commit into
livekit:mainfrom
Abhishek-B-R:fix/deferrable-map-abort-unhandled-rejection

Conversation

@Abhishek-B-R

Copy link
Copy Markdown

Summary

DeferrableMap.getDeferred(key, signal) (exposed as remoteParticipant.dataTracks.getDeferred()) removes its abort listener with future.promise.finally(...). .finally() returns a new promise that re-rejects with the same reason, and nothing handles that one. So when the signal aborts, the caller gets the AbortError on the promise it awaited, and a second copy of the same error is reported as an unhandled rejection. Wrapping the call in try/catch doesn't help, so apps that cancel a wait for a data track see "Uncaught (in promise) AbortError" in the console, and error trackers that listen for unhandledrejection record it.

Changes

  • Remove the listener with .then(cleanup, cleanup) instead of .finally(cleanup), so the internal chain handles both outcomes. The promise returned to the caller is unchanged and still rejects with DeferrableMapAbortError.
  • Adds src/utils/deferrable-map.test.ts (there was no test file for DeferrableMap): existing key, resolve on set, already aborted signal, and abort while pending.
  • Adds a patch changeset.

Testing

The abort test catches the rejection, waits one macrotask, then checks process.on('unhandledRejection') saw nothing. On main it fails with one unhandled AbortError (reason 'gave up'); with the fix all 4 tests pass. pnpm test passes (57 files, 977 tests), and tsc --noEmit, eslint and prettier are clean.

@changeset-bot

changeset-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 91385fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
livekit-client Patch

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

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@1egoman 1egoman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me, great catch and thank you for the contribution.

@Abhishek-B-R Would you be able to click the "CLA not signed yet" button and sign the CLA? Once you do this I'll merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants