Skip to content

Fix: Only call method channels on native platforms#1196

Merged
denrase merged 9 commits into
mainfrom
fix/method-channels-ios
Jan 9, 2023
Merged

Fix: Only call method channels on native platforms#1196
denrase merged 9 commits into
mainfrom
fix/method-channels-ios

Conversation

@denrase

@denrase denrase commented Dec 19, 2022

Copy link
Copy Markdown
Collaborator

📜 Description

Closes #1176

💡 Motivation and Context

  • Only call native cannels on native platforms
  • Also call native scope observers on iOS & macOS

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@codecov-commenter

codecov-commenter commented Dec 19, 2022 •
edited by codecov Bot
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.45%. Comparing base (9928a74) to head (1541762).
⚠️ Report is 1426 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (9928a74) and HEAD (1541762). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (9928a74) HEAD (1541762)
4 3
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #1196       +/-   ##
===========================================
- Coverage   88.87%   76.45%   -12.42%     
===========================================
  Files         121       11      -110     
  Lines        3810      327     -3483     
===========================================
- Hits         3386      250     -3136     
+ Misses        424       77      -347

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions

github-actions Bot commented Dec 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 320.88 ms 384.14 ms 63.26 ms
Size 5.94 MiB 6.96 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ccc09e4 308.21 ms 357.74 ms 49.54 ms
aed5947 295.70 ms 348.18 ms 52.48 ms
fbf42af 323.04 ms 365.09 ms 42.04 ms
cdf7172 348.54 ms 390.81 ms 42.27 ms
2f8f173 323.31 ms 373.29 ms 49.97 ms
3e5ee37 317.56 ms 366.84 ms 49.28 ms
559d28f 302.35 ms 339.53 ms 37.18 ms
72dfc83 298.62 ms 340.14 ms 41.52 ms
ae02632 309.16 ms 348.98 ms 39.82 ms
eb1a7c1 332.98 ms 381.55 ms 48.57 ms

App size

Revision Plain With Sentry Diff
ccc09e4 5.94 MiB 6.95 MiB 1.01 MiB
aed5947 5.94 MiB 6.96 MiB 1.02 MiB
fbf42af 5.94 MiB 6.96 MiB 1.02 MiB
cdf7172 5.94 MiB 6.95 MiB 1.01 MiB
2f8f173 5.94 MiB 6.95 MiB 1.01 MiB
3e5ee37 5.94 MiB 6.92 MiB 1001.19 KiB
559d28f 5.94 MiB 6.92 MiB 1001.70 KiB
72dfc83 5.94 MiB 6.92 MiB 1001.71 KiB
ae02632 5.94 MiB 6.95 MiB 1.01 MiB
eb1a7c1 5.94 MiB 6.92 MiB 1005.76 KiB

Previous results on branch: fix/method-channels-ios

Startup times

Revision Plain With Sentry Diff
50bb8f1 360.83 ms 432.41 ms 71.58 ms
bcb98b2 344.80 ms 383.92 ms 39.12 ms
1856064 314.94 ms 369.42 ms 54.48 ms
f181dc0 340.57 ms 394.32 ms 53.75 ms

App size

Revision Plain With Sentry Diff
50bb8f1 5.94 MiB 6.96 MiB 1.02 MiB
bcb98b2 5.94 MiB 6.96 MiB 1.02 MiB
1856064 5.94 MiB 6.96 MiB 1.02 MiB
f181dc0 5.94 MiB 6.96 MiB 1.02 MiB

@github-actions

github-actions Bot commented Dec 19, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1257.53 ms 1280.59 ms 23.06 ms
Size 8.16 MiB 9.17 MiB 1.01 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
3f23617 1261.93 ms 1286.10 ms 24.17 ms
1131914 1277.20 ms 1300.20 ms 23.00 ms
6325c3b 1266.52 ms 1291.06 ms 24.54 ms
613760b 1263.10 ms 1277.27 ms 14.16 ms
ef2f368 1259.12 ms 1277.04 ms 17.92 ms
eecbbca 1264.90 ms 1286.33 ms 21.43 ms
691aa3b 1265.57 ms 1282.13 ms 16.55 ms
0ceb89c 1252.02 ms 1271.78 ms 19.75 ms
62dde43 1258.43 ms 1276.81 ms 18.38 ms
a609134 1254.50 ms 1265.08 ms 10.58 ms

App size

Revision Plain With Sentry Diff
3f23617 8.16 MiB 9.17 MiB 1.01 MiB
1131914 8.16 MiB 9.17 MiB 1.01 MiB
6325c3b 8.16 MiB 9.17 MiB 1.01 MiB
613760b 8.15 MiB 9.13 MiB 1000.46 KiB
ef2f368 8.15 MiB 9.10 MiB 965.24 KiB
eecbbca 8.15 MiB 9.10 MiB 965.26 KiB
691aa3b 8.16 MiB 9.17 MiB 1.01 MiB
0ceb89c 8.15 MiB 9.12 MiB 989.78 KiB
62dde43 8.16 MiB 9.17 MiB 1.01 MiB
a609134 8.16 MiB 9.16 MiB 1.01 MiB

Previous results on branch: fix/method-channels-ios

Startup times

Revision Plain With Sentry Diff
f181dc0 1271.37 ms 1292.42 ms 21.05 ms
bcb98b2 1266.35 ms 1277.86 ms 11.50 ms
1856064 1273.82 ms 1292.48 ms 18.66 ms
50bb8f1 1264.76 ms 1276.58 ms 11.83 ms

App size

Revision Plain With Sentry Diff
f181dc0 8.16 MiB 9.17 MiB 1.01 MiB
bcb98b2 8.16 MiB 9.17 MiB 1.01 MiB
1856064 8.16 MiB 9.17 MiB 1.01 MiB
50bb8f1 8.16 MiB 9.17 MiB 1.01 MiB

@denrase
denrase marked this pull request as ready for review December 19, 2022 10:35
@denrase denrase mentioned this pull request Dec 19, 2022
Closed
5 tasks
marandaneto
marandaneto reviewed Dec 20, 2022
View reviewed changes
Comment thread flutter/test/sentry_flutter_util.dart
marandaneto
marandaneto approved these changes Dec 20, 2022
View reviewed changes

@marandaneto marandaneto 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.

Left a comment otherwise LGTM.

@denrase
denrase requested a review from marandaneto December 20, 2022 10:07
marandaneto
marandaneto reviewed Dec 20, 2022
View reviewed changes
Comment thread CHANGELOG.md
@marandaneto

marandaneto commented Dec 20, 2022

Copy link
Copy Markdown
Contributor

@denrase have you checked that the scope sync is working for iOS/macOS now? we need to be sure before going ahead.

@marandaneto

marandaneto commented Jan 9, 2023

Copy link
Copy Markdown
Contributor

@denrase have you checked that the scope sync is working for iOS/macOS now? we need to be sure before going ahead.

@denrase is this tested? ready to be merged?

@denrase

denrase commented Jan 9, 2023

Copy link
Copy Markdown
Collaborator Author

@marandaneto Set a sample user through scope sync in the example app and let it crash in swift code, user was synced. On the main branch, there is no user with the same code.

Here

Bildschirmfoto 2023-01-09 um 12 55 40

Main

Bildschirmfoto 2023-01-09 um 13 04 21

@denrase

denrase commented Jan 9, 2023

Copy link
Copy Markdown
Collaborator Author

@marandaneto Also works on macOS.

Bildschirmfoto 2023-01-09 um 13 14 41

@denrase

denrase commented Jan 9, 2023

Copy link
Copy Markdown
Collaborator Author

Tested with all methods. Issue with removeContexts and removeTag, which may be an issue in the cocoa SDK.

@denrase
denrase merged commit 3141b75 into main Jan 9, 2023
@denrase
denrase deleted the fix/method-channels-ios branch January 9, 2023 13:17
@denrase denrase mentioned this pull request Feb 14, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brustolin brustolin Awaiting requested review from brustolin
@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich
1 more reviewer
@marandaneto marandaneto marandaneto approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MethodChannel is called for platforms that dont have some implementations such as endNativeFrames

3 participants

@denrase @codecov-commenter @marandaneto