feat(core): Use native btoa for envelope base64 encoding#6351
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Use native btoa for envelope base64 encoding ([#6351](https://github.com/getsentry/sentry-react-native/pull/6351))If none of the above apply, you can opt out of this check by adding |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 31680e8. Configure here.
There was a problem hiding this comment.
nit: Not sure if this is really testable but if it is it would be nice to test both the btoa available/unavailable cases
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
Added two nits but LGTM 🚀
|
|
||
| /** | ||
| * Encodes a byte array to base64. Uses the runtime's native `btoa` when | ||
| * available (Hermes and modern JSC both expose it), which is significantly |
There was a problem hiding this comment.
nit: I think JSC does not always expose btoa
Hermes and modern JSC both expose a native `btoa`, which is significantly faster than the bundled JS encoder for the envelope payloads going through `RNSentry.captureEnvelope`. Convert the envelope `Uint8Array` to a binary string in 32KB chunks (to keep `String.fromCharCode.apply` argument count below engine limits) and feed it to `btoa`. Falls back to the existing `base64-js`-derived encoder when `btoa` is unavailable. Closes #4884.
- Soften the JSDoc claim about JSC: not every JSC build exposes `btoa`, only some do. - Add a test covering the JS-encoder fallback path by stashing `globalThis.btoa` so the module sees no native implementation.
alwx
force-pushed
the
alwx/feature/base64-new
branch
from
9774389 to
48a0f85
Compare
2 participants
📢 Type of change
📜 Description
Switch envelope base64 encoding on the
RNSentry.captureEnvelopehot path from the bundled JS encoder to the runtime's nativebtoa. Hermes and modern JSC both exposebtoa, and the native implementation should be significantly faster than the pure-JS encoder for the envelope payloads going through the bridge (profiles, attachments, replays).The comes with no native code and no breaking changes for the customers.
💡 Motivation and Context
Closes #4884.
Alternative approach to #6314 (which should be closed now)
💚 How did you test it?
test/utils/base64.test.ts📝 Checklist
sendDefaultPIIis enabled