Skip to content

Add TextDecoder fallback when fatal is unsupported - #2037

Merged
1egoman merged 6 commits into
livekit:mainfrom
JoelTowell:fix-text-decoder-issue
Aug 4, 2026
Merged

Add TextDecoder fallback when fatal is unsupported#2037
1egoman merged 6 commits into
livekit:mainfrom
JoelTowell:fix-text-decoder-issue

Conversation

@JoelTowell

@JoelTowell JoelTowell commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

TextStreamReader currently constructs TextDecoder('utf-8', { fatal: true }) unconditionally. I ran into an issue with React Native where TextDecoder is present, but constructing it with fatal: true throws:

Failed to construct 'TextDecoder': the 'fatal' option is unsupported

As a result, text stream handling fails before any decoding happens. This may not be isolated to one environment, as Expo explicitly notes that "[t]he TextDecoder API is not spec-compliant on native platforms".

The LiveKit React Native SDK uses a polyfill for TextDecoder (1, 2), but this will only take effect if TextDecoder is not already defined.

Based on discussion, change has been simplified from fallback to omitting the offending option.

Changes

  • Stop passing fatal: true when constructing TextDecoder in TextStreamReaderand bytesToDecodedUtf8 function

@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a48e60c

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

CLAassistant commented Aug 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@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 potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

@xianshijing-lk xianshijing-lk 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.

it looks good to me, and will get @1egoman or @lukasIO to take a final look

@JoelTowell

Copy link
Copy Markdown
Contributor Author

it looks good to me, and will get @1egoman or @lukasIO to take a final look

Thanks @xianshijing-lk ! I addressed the comment by Devin before I saw your comment. Happy to revert those changes if they are unwanted, though they make sense to me.

@1egoman

1egoman commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@JoelTowell Good catch here! I think this can be even simpler - you probably can just remove fatal: true outright rather than add all the additional switching logic. In practice, the bytes of a text data stream should always be valid utf-8 (according to the protocol) and adding all the switching logic introduces a lot of extra complexity.

@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 found 1 new potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment thread src/room/data-stream/incoming/StreamReader.ts
@JoelTowell

Copy link
Copy Markdown
Contributor Author

@JoelTowell Good catch here! I think this can be even simpler - you probably can just remove fatal: true outright rather than add all the additional switching logic. In practice, the bytes of a text data stream should always be valid utf-8 (according to the protocol) and adding all the switching logic introduces a lot of extra complexity.

Thanks @1egoman! I reverted my changes and just stopped passing fatal: true to the constructor. I updated the PR description as well. Devin doesn't seem thrilled about it, but I'm certain you're a better judge. Should be good to merge. 🙂

@1egoman

1egoman commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Yep I think it should be fine in practice. Thanks for the contribution!

@1egoman
1egoman merged commit cbee8ad into livekit:main Aug 4, 2026
6 checks passed
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.

4 participants