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.

Sign up for GitHub

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

Jump to bottom

fix(timetodisplay): Change TimeToDisplay unsupported message from error to warning #3699

Merged
merged 5 commits into from
Apr 16, 2024
Merged

fix(timetodisplay): Change TimeToDisplay unsupported message from error to warning #3699

merged 5 commits into from
Apr 16, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Mar 20, 2024 •
edited
Loading

📢 Type of change

  • Enhancement
  • Refactoring

📜 Description

The error log will trigger a stack trace log in React which is an unwanted side-effect. The log then looks like a crash/uncaught error. Plus the stack trace is not symbolicated so it's not useful.

This PR changes the log to a warning and uses Timeout to avoid the React stack printing.

Before:
Screenshot 2024-03-20 at 19 16 30

After:
Screenshot 2024-03-21 at 16 37 44
Screenshot 2024-03-21 at 16 37 39

💚 How did you test it?

sample app

📝 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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 20, 2024 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 472.77 ms 482.94 ms 10.17 ms
Size 17.73 MiB 19.93 MiB 2.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 333.78 ms 343.94 ms 10.16 ms
6e8584e 447.10 ms 474.71 ms 27.61 ms
31fcca2 391.22 ms 414.78 ms 23.56 ms
5571a20 410.55 ms 441.06 ms 30.51 ms
d361d38 354.10 ms 381.69 ms 27.59 ms
e73f4ed+dirty 332.96 ms 354.33 ms 21.37 ms
2534337 394.15 ms 415.12 ms 20.97 ms
457e29f 398.10 ms 421.39 ms 23.29 ms
5a22220 412.38 ms 447.35 ms 34.97 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 17.73 MiB 19.75 MiB 2.02 MiB
6e8584e 17.73 MiB 19.86 MiB 2.12 MiB
31fcca2 17.73 MiB 19.90 MiB 2.17 MiB
5571a20 17.73 MiB 19.93 MiB 2.19 MiB
d361d38 17.73 MiB 19.81 MiB 2.08 MiB
e73f4ed+dirty 17.73 MiB 20.04 MiB 2.31 MiB
2534337 17.73 MiB 19.84 MiB 2.11 MiB
457e29f 17.73 MiB 19.84 MiB 2.10 MiB
5a22220 17.73 MiB 19.93 MiB 2.20 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 20, 2024 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1226.51 ms 1240.75 ms 14.24 ms
Size 2.36 MiB 2.92 MiB 569.91 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ad6c299+dirty 1244.76 ms 1260.10 ms 15.34 ms
0db0c72+dirty 1275.02 ms 1285.84 ms 10.82 ms
d0bf494+dirty 1289.40 ms 1298.40 ms 9.00 ms
76d1baf+dirty 1244.10 ms 1268.52 ms 24.42 ms
f06c879+dirty 1252.64 ms 1259.66 ms 7.02 ms
22e31b6+dirty 1253.62 ms 1265.96 ms 12.34 ms
9a3ca65+dirty 1247.06 ms 1274.58 ms 27.52 ms
b1e8712+dirty 1256.02 ms 1265.14 ms 9.12 ms
34aba08+dirty 1276.78 ms 1308.52 ms 31.74 ms
70caa60+dirty 1218.27 ms 1230.30 ms 12.03 ms

App size

Revision Plain With Sentry Diff
ad6c299+dirty 2.36 MiB 2.84 MiB 488.85 KiB
0db0c72+dirty 2.36 MiB 2.84 MiB 487.01 KiB
d0bf494+dirty 2.36 MiB 2.83 MiB 481.15 KiB
76d1baf+dirty 2.36 MiB 2.82 MiB 469.45 KiB
f06c879+dirty 2.36 MiB 2.88 MiB 530.42 KiB
22e31b6+dirty 2.36 MiB 2.87 MiB 520.67 KiB
9a3ca65+dirty 2.36 MiB 2.82 MiB 462.89 KiB
b1e8712+dirty 2.36 MiB 2.84 MiB 488.84 KiB
34aba08+dirty 2.36 MiB 2.85 MiB 495.32 KiB
70caa60+dirty 2.36 MiB 2.83 MiB 479.27 KiB

@github-actions GitHub Actions
Copy link
Contributor

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1228.10 ms 1229.06 ms 0.96 ms
Size 2.92 MiB 3.48 MiB 575.50 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
80b2ce3+dirty 1245.12 ms 1262.04 ms 16.92 ms
d7401ac+dirty 1288.10 ms 1289.54 ms 1.44 ms
e5c9b8b+dirty 1276.90 ms 1280.92 ms 4.02 ms
acadc0f+dirty 1271.12 ms 1272.28 ms 1.16 ms
728164b+dirty 1280.06 ms 1285.26 ms 5.20 ms
3ffcddd+dirty 1272.22 ms 1273.98 ms 1.76 ms
9a3ca65+dirty 1276.40 ms 1279.14 ms 2.74 ms
457e29f+dirty 1256.71 ms 1258.50 ms 1.79 ms
12427f4+dirty 1224.90 ms 1231.40 ms 6.50 ms
2534337+dirty 1220.87 ms 1221.47 ms 0.60 ms

App size

Revision Plain With Sentry Diff
80b2ce3+dirty 2.92 MiB 3.40 MiB 492.75 KiB
d7401ac+dirty 2.92 MiB 3.40 MiB 488.06 KiB
e5c9b8b+dirty 2.92 MiB 3.43 MiB 524.50 KiB
acadc0f+dirty 2.92 MiB 3.39 MiB 487.34 KiB
728164b+dirty 2.92 MiB 3.44 MiB 533.26 KiB
3ffcddd+dirty 2.92 MiB 3.40 MiB 494.39 KiB
9a3ca65+dirty 2.92 MiB 3.37 MiB 464.32 KiB
457e29f+dirty 2.92 MiB 3.43 MiB 524.75 KiB
12427f4+dirty 2.92 MiB 3.44 MiB 533.29 KiB
2534337+dirty 2.92 MiB 3.43 MiB 529.76 KiB

@krystofwoldrich krystofwoldrich marked this pull request as ready for review March 21, 2024 15:39
@krystofwoldrich krystofwoldrich changed the title fix(timetodisplay): Change TimeToDisplay unsupported log from error t… fix(timetodisplay): Change TimeToDisplay unsupported message from error to warning Mar 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 21, 2024 •
edited
Loading

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 388.57 ms 421.71 ms 33.14 ms
Size 7.15 MiB 8.21 MiB 1.06 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 363.19 ms 370.37 ms 7.18 ms
34aba08+dirty 331.79 ms 376.69 ms 44.91 ms
0db0c72+dirty 335.20 ms 351.06 ms 15.86 ms
12427f4+dirty 379.48 ms 400.92 ms 21.44 ms
5a22220+dirty 384.61 ms 419.06 ms 34.45 ms
27ef4ee+dirty 296.71 ms 351.00 ms 54.29 ms
e73f4ed+dirty 262.98 ms 311.02 ms 48.04 ms
b1e8712+dirty 322.55 ms 331.84 ms 9.29 ms
d7401ac+dirty 373.98 ms 394.08 ms 20.10 ms
76d1baf+dirty 339.02 ms 408.65 ms 69.63 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 7.15 MiB 8.04 MiB 910.84 KiB
34aba08+dirty 7.15 MiB 8.07 MiB 946.13 KiB
0db0c72+dirty 7.15 MiB 8.04 MiB 911.02 KiB
12427f4+dirty 7.15 MiB 8.12 MiB 997.78 KiB
5a22220+dirty 7.15 MiB 8.21 MiB 1.06 MiB
27ef4ee+dirty 7.15 MiB 8.08 MiB 959.49 KiB
e73f4ed+dirty 7.15 MiB 8.09 MiB 965.94 KiB
b1e8712+dirty 7.15 MiB 8.04 MiB 912.27 KiB
d7401ac+dirty 7.15 MiB 8.04 MiB 910.85 KiB
76d1baf+dirty 7.15 MiB 8.09 MiB 964.41 KiB

lucas-zimerman
lucas-zimerman approved these changes Apr 4, 2024
View reviewed changes
Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

LGTM! (added a nit comment in the code for future reference)

@krystofwoldrich krystofwoldrich merged commit a9dd3fc into main Apr 16, 2024
49 of 51 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-ttd-error-to-warning branch April 16, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucas-zimerman lucas-zimerman lucas-zimerman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krystofwoldrich @lucas-zimerman