Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Linked errors processed before other integrations #3535

Merged
merged 12 commits into from
Jan 25, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Jan 18, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Javascript SDK 7.74.0 introduced new refactored integrations. These use new preprocessEvent and processEvent hooks replacing the globalEventProcessor.

This changes the order in which the integrations process the events. For RN SDK this results in frames being rewritten first and then linked errors are added and never rewritten.

Related JS PRs:

This PR updates the NativeLinkedErrors to use preprocessEvent, ensuring it runs before all other processEvent and globalEventProcessor integrations.

This PR also adds new tests, verifying the execution order.

💚 How did you test it?

integration 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 - BUT We are changing the internal implementation of NativeLinkedErrors class.

🔮 Next steps

Copy link
Contributor

github-actions bot commented Jan 18, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 406.04 ms 441.65 ms 35.61 ms
Size 17.73 MiB 19.84 MiB 2.11 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2534337 394.15 ms 415.12 ms 20.97 ms
e2b64fe 316.88 ms 330.23 ms 13.35 ms
12427f4 393.69 ms 414.84 ms 21.14 ms
d361d38 354.10 ms 381.69 ms 27.59 ms
acadc0f+dirty 373.24 ms 381.51 ms 8.27 ms
e73f4ed+dirty 332.96 ms 354.33 ms 21.37 ms
abb7058 370.27 ms 389.58 ms 19.31 ms
34aba08 328.10 ms 342.84 ms 14.74 ms
457e29f 398.10 ms 421.39 ms 23.29 ms
22e31b6 396.48 ms 419.64 ms 23.16 ms

App size

Revision Plain With Sentry Diff
2534337 17.73 MiB 19.84 MiB 2.11 MiB
e2b64fe 17.73 MiB 19.80 MiB 2.07 MiB
12427f4 17.73 MiB 19.85 MiB 2.12 MiB
d361d38 17.73 MiB 19.81 MiB 2.08 MiB
acadc0f+dirty 17.73 MiB 19.75 MiB 2.01 MiB
e73f4ed+dirty 17.73 MiB 20.04 MiB 2.31 MiB
abb7058 17.73 MiB 19.83 MiB 2.10 MiB
34aba08 17.73 MiB 19.80 MiB 2.07 MiB
457e29f 17.73 MiB 19.84 MiB 2.10 MiB
22e31b6 17.73 MiB 19.84 MiB 2.10 MiB

Copy link
Contributor

github-actions bot commented Jan 18, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 455.80 ms 504.38 ms 48.57 ms
Size 7.15 MiB 8.11 MiB 989.81 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
12427f4+dirty 379.48 ms 400.92 ms 21.44 ms
5446992+dirty 371.61 ms 390.00 ms 18.39 ms
1d86dd6+dirty 335.76 ms 371.22 ms 35.46 ms
b1e8712+dirty 322.55 ms 331.84 ms 9.29 ms
9433f35+dirty 265.50 ms 336.08 ms 70.58 ms
3ffcddd+dirty 244.82 ms 295.47 ms 50.65 ms
acadc0f+dirty 259.04 ms 304.67 ms 45.63 ms
575f9da+dirty 337.15 ms 370.47 ms 33.32 ms
e73f4ed+dirty 262.98 ms 311.02 ms 48.04 ms
34aba08+dirty 331.79 ms 376.69 ms 44.91 ms

App size

Revision Plain With Sentry Diff
12427f4+dirty 7.15 MiB 8.12 MiB 997.78 KiB
5446992+dirty 7.15 MiB 8.12 MiB 999.45 KiB
1d86dd6+dirty 7.15 MiB 8.13 MiB 1002.18 KiB
b1e8712+dirty 7.15 MiB 8.04 MiB 912.27 KiB
9433f35+dirty 7.15 MiB 8.08 MiB 959.34 KiB
3ffcddd+dirty 7.15 MiB 8.04 MiB 912.17 KiB
acadc0f+dirty 7.15 MiB 8.03 MiB 903.20 KiB
575f9da+dirty 7.15 MiB 8.10 MiB 979.68 KiB
e73f4ed+dirty 7.15 MiB 8.09 MiB 965.94 KiB
34aba08+dirty 7.15 MiB 8.07 MiB 946.13 KiB

Copy link
Contributor

github-actions bot commented Jan 20, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1206.50 ms 1211.14 ms 4.64 ms
Size 2.36 MiB 2.88 MiB 526.45 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9433f35+dirty 1246.94 ms 1271.45 ms 24.52 ms
9c48b2c+dirty 1246.96 ms 1255.73 ms 8.77 ms
1d86dd6+dirty 1249.71 ms 1279.16 ms 29.45 ms
d361d38+dirty 1246.04 ms 1267.12 ms 21.08 ms
12427f4+dirty 1267.15 ms 1271.30 ms 4.15 ms
6e8584e+dirty 1274.50 ms 1296.82 ms 22.32 ms
3ffcddd+dirty 1244.47 ms 1264.14 ms 19.67 ms
22e31b6+dirty 1253.62 ms 1265.96 ms 12.34 ms
0677344+dirty 1276.70 ms 1300.07 ms 23.37 ms
3853f43+dirty 1221.82 ms 1242.64 ms 20.82 ms

App size

Revision Plain With Sentry Diff
9433f35+dirty 2.36 MiB 2.85 MiB 499.80 KiB
9c48b2c+dirty 2.36 MiB 2.85 MiB 495.77 KiB
1d86dd6+dirty 2.36 MiB 2.89 MiB 535.43 KiB
d361d38+dirty 2.36 MiB 2.85 MiB 499.84 KiB
12427f4+dirty 2.36 MiB 2.88 MiB 530.38 KiB
6e8584e+dirty 2.36 MiB 2.88 MiB 533.17 KiB
3ffcddd+dirty 2.36 MiB 2.84 MiB 489.60 KiB
22e31b6+dirty 2.36 MiB 2.87 MiB 520.67 KiB
0677344+dirty 2.36 MiB 2.85 MiB 496.81 KiB
3853f43+dirty 2.36 MiB 2.85 MiB 499.81 KiB

@krystofwoldrich krystofwoldrich changed the title fix: Native Linked Errors integration has to be executed before Rewrite Frames fix: NativeLinkedErrors processes event before RewriteFrames Jan 22, 2024
@krystofwoldrich krystofwoldrich marked this pull request as ready for review January 22, 2024 12:51
@krystofwoldrich krystofwoldrich changed the title fix: NativeLinkedErrors processes event before RewriteFrames fix: Linked errors processed before other integrations Jan 22, 2024
@krystofwoldrich krystofwoldrich requested a review from mydea January 22, 2024 20:09
@krystofwoldrich krystofwoldrich requested a review from mydea January 23, 2024 17:20
@krystofwoldrich
Copy link
Member Author

krystofwoldrich commented Jan 23, 2024

DO NOT MERGE, I've found an issue with the async load of the native symbols.

The native symbols are fixed.

@krystofwoldrich
Copy link
Member Author

@mydea Thank you for the review, can you give it one more look?

@krystofwoldrich krystofwoldrich merged commit 58b3261 into main Jan 25, 2024
48 of 51 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-fix-linked-errors-symbolication branch January 25, 2024 05:08
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.

Linked errors are not symbolicated (RewriteFrames and LinkedErrors are not in correct order)
2 participants