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: Core data span status with error #2439

Merged
merged 3 commits into from Nov 29, 2022
Merged

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Nov 23, 2022

📜 Description

Checks core data request result to set span finish status.

💡 Motivation and Context

close #2434
close #2433

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@brustolin
Copy link
Contributor Author

@philipphofmann I haven't see #2433 before.
This problem can crash an Obj-c project.

@github-actions
Copy link

github-actions bot commented Nov 23, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1263.24 ms 1279.72 ms 16.48 ms
Size 20.75 KiB 374.75 KiB 353.99 KiB

Baseline results on branch: master

Startup times

Revision Plain With Sentry Diff
864c39a 1191.14 ms 1233.38 ms 42.24 ms
f781ecb 1213.76 ms 1228.54 ms 14.78 ms
0fdf0b2 1194.37 ms 1227.90 ms 33.53 ms
b45b682 1224.16 ms 1255.12 ms 30.96 ms
791123d 1256.10 ms 1266.68 ms 10.58 ms
074350f 1241.00 ms 1249.60 ms 8.60 ms
e2a3f3e 1234.37 ms 1257.13 ms 22.76 ms
b15627c 1228.88 ms 1269.70 ms 40.82 ms
6177f2d 1206.55 ms 1226.20 ms 19.65 ms
09ad77e 1242.14 ms 1244.02 ms 1.88 ms

App size

Revision Plain With Sentry Diff
864c39a 20.51 KiB 335.57 KiB 315.06 KiB
f781ecb 20.75 KiB 374.72 KiB 353.97 KiB
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB
b45b682 20.75 KiB 374.53 KiB 353.78 KiB
791123d 20.51 KiB 331.81 KiB 311.30 KiB
074350f 20.51 KiB 335.52 KiB 315.01 KiB
e2a3f3e 20.75 KiB 368.01 KiB 347.26 KiB
b15627c 20.50 KiB 337.76 KiB 317.25 KiB
6177f2d 20.51 KiB 332.90 KiB 312.40 KiB
09ad77e 20.50 KiB 339.01 KiB 318.51 KiB

@kevinrenskers
Copy link
Contributor

Shouldn't we merge this? If this is for the master branch that implies it was important enough for a hot fix release instead of adding it to 8.0.0.

@brustolin brustolin merged commit df5d8d7 into master Nov 29, 2022
@brustolin brustolin deleted the fix/coredata-errorreport branch November 29, 2022 11:46
kevinrenskers added a commit that referenced this pull request Nov 29, 2022
* 8.0.0:
  Merge branch 'master' into 8.0.0
  release: 7.31.3
  revert rename of sentryuser (#2462)
  fix: Reporting crashes when restarting the SDK (#2440)
  Update integration-tests.yml (#2461)
  fix: Core data span status with error (#2439)
  meta: disable swiftlint file length check in TBDBClient.swift (#2435)
  Revert "test: shorten some tests (#2422)" (#2427)
  test: shorten some tests (#2422)

# Conflicts:
#	Tests/SentryTests/Helper/TestNSNotificationCenterWrapper.swift
kevinrenskers added a commit that referenced this pull request Nov 29, 2022
…ERSIZE

* 8.0.0:
  fix: SentryAppStateManager should always stop when closing the SDK (#2460)
  Merge branch 'master' into 8.0.0
  release: 7.31.3
  fix: Reporting crashes when restarting the SDK (#2440)
  Update integration-tests.yml (#2461)
  fix: Core data span status with error (#2439)
  meta: disable swiftlint file length check in TBDBClient.swift (#2435)
  Revert "test: shorten some tests (#2422)" (#2427)
  test: shorten some tests (#2422)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants