fix(sdk): Transfer large envelopes (tens of MB) over bridge without crash #2852

krystofwoldrich · 2023-02-23T10:27:37Z

📢 Type of change

Bugfix
New feature
Enhancement
Refactoring

📜 Description

Arrays larger than 196603 will crash RN Bridge, params of the called function will disappear and Malformed JS Call error will be thrown. Since we support attachments 200KB can be easily reached.

The implementation was tested using generated large data.

const dataSize = 20e6; // 20MB
const simulatedData = new Uint8Array(dataSize);
for (let i = 0; i < dataSize; i++) {
  simulatedData[i] = i % 256;
}
scope.addAttachment({ data: simulatedData, filename: 'big.bin', contentType: 'application/octet-stream' });

Android simulator was sending 20MB successfully larger envelops started crashing the app (50MB and more)

iOS simulator was sending envelopes up to 60MB without problems.

💡 Motivation and Context

closes: #2744

💚 How did you test it?

sample app, unit tests

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
All tests passing
No breaking changes

🔮 Next steps

Add the patch to v4.

github-actions · 2023-02-23T11:07:39Z

iOS (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1225.16 ms	1248.36 ms	23.20 ms
Size	2.36 MiB	2.87 MiB	521.29 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	1289.40 ms	1298.40 ms	9.00 ms
`3853f43`+dirty	1221.82 ms	1242.64 ms	20.82 ms
`e73f4ed`+dirty	1243.27 ms	1244.52 ms	1.25 ms
`9c48b2c`+dirty	1246.96 ms	1255.73 ms	8.77 ms
`27ef4ee`+dirty	1293.52 ms	1296.08 ms	2.56 ms
`6e8584e`+dirty	1274.50 ms	1296.82 ms	22.32 ms
`acadc0f`+dirty	1264.38 ms	1290.06 ms	25.68 ms
`8900e1a`+dirty	1210.27 ms	1218.66 ms	8.39 ms
`e2b64fe`+dirty	1232.22 ms	1255.20 ms	22.98 ms
`e5c9b8b`+dirty	1258.57 ms	1267.32 ms	8.75 ms

App size

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	2.36 MiB	2.83 MiB	481.15 KiB
`3853f43`+dirty	2.36 MiB	2.85 MiB	499.81 KiB
`e73f4ed`+dirty	2.36 MiB	2.82 MiB	469.44 KiB
`9c48b2c`+dirty	2.36 MiB	2.85 MiB	495.77 KiB
`27ef4ee`+dirty	2.36 MiB	2.85 MiB	500.03 KiB
`6e8584e`+dirty	2.36 MiB	2.88 MiB	533.17 KiB
`acadc0f`+dirty	2.36 MiB	2.83 MiB	480.37 KiB
`8900e1a`+dirty	2.36 MiB	2.83 MiB	479.25 KiB
`e2b64fe`+dirty	2.36 MiB	2.85 MiB	495.80 KiB
`e5c9b8b`+dirty	2.36 MiB	2.87 MiB	520.43 KiB

github-actions · 2023-02-23T11:21:16Z

Android (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	564.89 ms	629.33 ms	64.44 ms
Size	7.15 MiB	8.11 MiB	987.40 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	253.73 ms	308.23 ms	54.49 ms
`3853f43`+dirty	278.12 ms	338.72 ms	60.60 ms
`e73f4ed`+dirty	262.98 ms	311.02 ms	48.04 ms
`9c48b2c`+dirty	270.82 ms	321.12 ms	50.30 ms
`27ef4ee`+dirty	296.71 ms	351.00 ms	54.29 ms
`6e8584e`+dirty	383.37 ms	400.84 ms	17.47 ms
`acadc0f`+dirty	259.04 ms	304.67 ms	45.63 ms
`8900e1a`+dirty	371.40 ms	377.70 ms	6.31 ms
`e2b64fe`+dirty	258.82 ms	304.26 ms	45.44 ms
`e5c9b8b`+dirty	335.40 ms	360.06 ms	24.67 ms

App size

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	7.15 MiB	8.04 MiB	910.85 KiB
`3853f43`+dirty	7.15 MiB	8.08 MiB	959.34 KiB
`e73f4ed`+dirty	7.15 MiB	8.09 MiB	965.94 KiB
`9c48b2c`+dirty	7.15 MiB	8.07 MiB	947.16 KiB
`27ef4ee`+dirty	7.15 MiB	8.08 MiB	959.49 KiB
`6e8584e`+dirty	7.15 MiB	8.13 MiB	1002.18 KiB
`acadc0f`+dirty	7.15 MiB	8.03 MiB	903.20 KiB
`8900e1a`+dirty	7.15 MiB	8.03 MiB	901.79 KiB
`e2b64fe`+dirty	7.15 MiB	8.07 MiB	947.16 KiB
`e5c9b8b`+dirty	7.15 MiB	8.10 MiB	980.41 KiB

marandaneto · 2023-03-02T09:31:33Z

@krystofwoldrich if the size is a problem, base64 is 1/3 bigger than the raw array of bytes by definition.
Also, transferring a String would be a problem for binary attachments, you need to be sure that the encoding is not going to break, for example, images, etc.
Is there any other solution here?

marandaneto · 2023-03-02T09:36:03Z

@krystofwoldrich if the size is a problem, base64 is 1/3 bigger than the raw array of bytes by definition.
Also, transferring a String would be a problem for binary attachments, you need to be sure that the encoding is not going to break, for example, images, etc.
Is there any other solution here?

krystofwoldrich · 2023-03-02T09:43:38Z

@marandaneto
The problem is array length not the size in bytes itself.

I've checked how FS libraries are handling this, as they also need to transfer large binary data and they do it the same.

https://github.com/itinance/react-native-fs/blob/master/FS.common.js#L396

marandaneto · 2023-03-02T09:54:24Z

@marandaneto The problem is array length not the size in bytes itself.

I've checked how FS libraries are handling this, as they also need to transfer large binary data and they do it the same.

itinance/react-native-fs@master/FS.common.js#L396

The thing is that the bigger, the more the overhead of serialization/deserialization to base64.
The description and title are about bigger in size and not in length, I am a bit confused now, can you elaborate?

krystofwoldrich · 2023-03-02T10:52:50Z

@marandaneto

196603 is the max array length that we can send over the bridge. Any larger array (no matter the array content) will disappear from the parameters causing the malformed JS call.

In our case, it's a bytes array so this length represents about 200 KB envelope.

krystofwoldrich · 2023-03-06T13:54:12Z

facebook/hermes#994

src/js/wrapper.ts

marandaneto · 2023-03-07T10:24:58Z

If this has to be fixed ASAP, go for it, otherwise, I'd consider trying to get feedback on facebook/hermes#994 just to be sure we are doing the right thing.

krystofwoldrich · 2023-03-07T12:26:39Z

It affects only Hermes dev builds, so I will wait for the facebook/hermes#994 resolution.
This fix would increase the bundle size.

github-actions · 2023-03-29T00:12:58Z

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

krystofwoldrich · 2023-04-07T09:59:13Z

Closing this as the bug only happens in dev env and by fixing facebook/hermes#994 in Hermes, there should not be any changes required on our end.

…pe-malformed-js-call

github-actions · 2023-11-27T19:32:56Z

Android (legacy) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	416.82 ms	421.39 ms	4.57 ms
Size	17.73 MiB	19.84 MiB	2.11 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`e5c9b8b`	409.02 ms	426.66 ms	17.64 ms
`d0bf494`+dirty	375.37 ms	395.14 ms	19.77 ms
`575f9da`	415.26 ms	422.98 ms	7.72 ms
`0db0c72`	372.12 ms	386.00 ms	13.88 ms
`e73f4ed`+dirty	332.96 ms	354.33 ms	21.37 ms
`acadc0f`+dirty	373.24 ms	381.51 ms	8.27 ms
`8900e1a`+dirty	430.68 ms	456.13 ms	25.44 ms
`3ffcddd`	302.92 ms	315.80 ms	12.88 ms
`d361d38`	354.10 ms	381.69 ms	27.59 ms
`f06c879`	408.41 ms	424.54 ms	16.13 ms

App size

Revision	Plain	With Sentry	Diff
`e5c9b8b`	17.73 MiB	19.83 MiB	2.10 MiB
`d0bf494`+dirty	17.73 MiB	19.75 MiB	2.02 MiB
`575f9da`	17.73 MiB	19.83 MiB	2.10 MiB
`0db0c72`	17.73 MiB	19.75 MiB	2.02 MiB
`e73f4ed`+dirty	17.73 MiB	20.04 MiB	2.31 MiB
`acadc0f`+dirty	17.73 MiB	19.75 MiB	2.01 MiB
`8900e1a`+dirty	17.73 MiB	19.75 MiB	2.01 MiB
`3ffcddd`	17.73 MiB	19.75 MiB	2.02 MiB
`d361d38`	17.73 MiB	19.81 MiB	2.08 MiB
`f06c879`	17.73 MiB	19.85 MiB	2.12 MiB

Previous results on branch: kw-fix-capture-envelope-malformed-js-call

Startup times

Revision	Plain	With Sentry	Diff
`aa5217b`	443.60 ms	468.75 ms	25.15 ms

App size

Revision	Plain	With Sentry	Diff
`aa5217b`	17.73 MiB	19.84 MiB	2.11 MiB

github-actions · 2023-11-27T19:43:10Z

iOS (new) Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1286.86 ms	1298.72 ms	11.86 ms
Size	2.92 MiB	3.43 MiB	524.90 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	1266.20 ms	1267.52 ms	1.32 ms
`3853f43`+dirty	1271.74 ms	1278.04 ms	6.30 ms
`e73f4ed`+dirty	1282.90 ms	1309.30 ms	26.40 ms
`9c48b2c`+dirty	1253.39 ms	1256.30 ms	2.91 ms
`27ef4ee`+dirty	1236.41 ms	1244.90 ms	8.49 ms
`6e8584e`+dirty	1271.71 ms	1281.26 ms	9.55 ms
`acadc0f`+dirty	1271.12 ms	1272.28 ms	1.16 ms
`8900e1a`+dirty	1268.36 ms	1273.04 ms	4.68 ms
`e2b64fe`+dirty	1285.78 ms	1297.56 ms	11.78 ms
`e5c9b8b`+dirty	1276.90 ms	1280.92 ms	4.02 ms

App size

Revision	Plain	With Sentry	Diff
`d0bf494`+dirty	2.92 MiB	3.40 MiB	488.08 KiB
`3853f43`+dirty	2.92 MiB	3.41 MiB	503.54 KiB
`e73f4ed`+dirty	2.92 MiB	3.38 MiB	475.71 KiB
`9c48b2c`+dirty	2.92 MiB	3.41 MiB	499.97 KiB
`27ef4ee`+dirty	2.92 MiB	3.41 MiB	503.72 KiB
`6e8584e`+dirty	2.92 MiB	3.44 MiB	536.52 KiB
`acadc0f`+dirty	2.92 MiB	3.39 MiB	487.34 KiB
`8900e1a`+dirty	2.92 MiB	3.39 MiB	485.96 KiB
`e2b64fe`+dirty	2.92 MiB	3.41 MiB	499.97 KiB
`e5c9b8b`+dirty	2.92 MiB	3.43 MiB	524.50 KiB

Previous results on branch: kw-fix-capture-envelope-malformed-js-call

Startup times

Revision	Plain	With Sentry	Diff
`aa5217b`+dirty	1301.86 ms	1312.72 ms	10.86 ms

App size

Revision	Plain	With Sentry	Diff
`aa5217b`+dirty	2.92 MiB	3.43 MiB	524.92 KiB

krystofwoldrich · 2023-11-29T09:19:03Z

With the recent addition of native profiling, the SDK crashes on the byte array size limit more often than before.

Therefore we should address this.

ios/RNSentry.mm

src/js/vendor/base64-js/fromByteArray.ts

lucas-zimerman

LGTM!

yogendrajs · 2023-12-04T07:38:28Z

@krystofwoldrich & team: Thanks for merging this, when will this be released? Do you know if there is a tag that I can use while on?

kahest · 2023-12-19T09:06:24Z

@yogendrajs this was released with version 5.15.0

krystofwoldrich added 6 commits February 23, 2023 10:25

Add base64-js vendor

d51c7fc

Change Spec of NativeRNSentry

f5aab6e

Use typed array to store envelope bytes

d020d9f

Switch android to base64 envelopes

88392c1

Switch iOS to base64 envelopes

00dbaef

Add changelog

a704d11

krystofwoldrich requested a review from marandaneto as a code owner February 23, 2023 10:27

krystofwoldrich added 2 commits February 23, 2023 11:29

Fix changelog pr id

272756f

Update changelog

0c87592

marandaneto reviewed Mar 7, 2023

View reviewed changes

src/js/wrapper.ts Outdated Show resolved Hide resolved

github-actions bot added the Status: Stale label Mar 29, 2023

marandaneto added Status: Backlog Blocked and removed Status: Stale labels Mar 29, 2023

krystofwoldrich closed this Apr 7, 2023

krystofwoldrich reopened this May 15, 2023

krystofwoldrich removed the Blocked label May 15, 2023

Merge remote-tracking branch 'origin/main' into kw-fix-capture-envelo…

e68254c

…pe-malformed-js-call

getsantry bot added the Stale label Oct 18, 2023

krystofwoldrich removed the Stale label Oct 18, 2023

Merge branch 'main' into kw-fix-capture-envelope-malformed-js-call

0884da1

krystofwoldrich requested a review from lucas-zimerman as a code owner November 27, 2023 18:53

krystofwoldrich added 4 commits November 27, 2023 20:12

fix tests bad merge

3e4b1f2

fix changelog

d438138

fix license

f1da7fb

fix lint

da0d596

krystofwoldrich closed this Nov 27, 2023

krystofwoldrich reopened this Nov 27, 2023

add js docs

0fe88c0

krystofwoldrich requested review from markushi, philipphofmann, Lms24 and AbhiPrasad and removed request for marandaneto November 28, 2023 14:36

lucas-zimerman reviewed Nov 29, 2023

View reviewed changes

ios/RNSentry.mm Outdated Show resolved Hide resolved

lucas-zimerman reviewed Nov 29, 2023

View reviewed changes

src/js/vendor/base64-js/fromByteArray.ts Outdated Show resolved Hide resolved

lucas-zimerman reviewed Nov 29, 2023

View reviewed changes

src/js/vendor/base64-js/fromByteArray.ts Outdated Show resolved Hide resolved

fix review comments

449c399

krystofwoldrich requested a review from lucas-zimerman November 29, 2023 15:24

lucas-zimerman approved these changes Nov 29, 2023

View reviewed changes

krystofwoldrich merged commit f15994e into main Nov 30, 2023
59 of 61 checks passed

krystofwoldrich deleted the kw-fix-capture-envelope-malformed-js-call branch November 30, 2023 11:53

kahest mentioned this pull request Dec 19, 2023

App Crashes with NSInvalidArgumentException Invalid number value (infinite) in JSON write #3421

Closed

11 tasks

fix(sdk): Transfer large envelopes (tens of MB) over bridge without crash #2852

fix(sdk): Transfer large envelopes (tens of MB) over bridge without crash #2852

Conversation

krystofwoldrich commented Feb 23, 2023 • edited Loading

📢 Type of change

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Feb 23, 2023 • edited Loading

iOS (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

github-actions bot commented Feb 23, 2023 • edited Loading

Android (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

marandaneto commented Mar 2, 2023

marandaneto commented Mar 2, 2023

krystofwoldrich commented Mar 2, 2023

marandaneto commented Mar 2, 2023

krystofwoldrich commented Mar 2, 2023 • edited Loading

krystofwoldrich commented Mar 6, 2023

marandaneto commented Mar 7, 2023

krystofwoldrich commented Mar 7, 2023

github-actions bot commented Mar 29, 2023

krystofwoldrich commented Apr 7, 2023

github-actions bot commented Nov 27, 2023 • edited Loading

Android (legacy) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: kw-fix-capture-envelope-malformed-js-call

Startup times

App size

github-actions bot commented Nov 27, 2023 • edited Loading

iOS (new) Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: kw-fix-capture-envelope-malformed-js-call

Startup times

App size

krystofwoldrich commented Nov 29, 2023

lucas-zimerman left a comment

Choose a reason for hiding this comment

yogendrajs commented Dec 4, 2023

kahest commented Dec 19, 2023

krystofwoldrich commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

github-actions bot commented Feb 23, 2023 •

edited

Loading

krystofwoldrich commented Mar 2, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading

github-actions bot commented Nov 27, 2023 •

edited

Loading