Skip to content
New issue

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

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

Already on GitHub? # to your account

[Beta fix] Update Sentry to avoid crash in SDK init #13828

Merged

Conversation

joshheald
Copy link
Contributor

@joshheald joshheald commented Sep 3, 2024

Closes: #13824

Description

Sentry found a crash in their SDK initialisation process: getsentry/sentry-cocoa#4280

It’s difficult to assess the impact, as we don’t have the benefit of any Sentry issues for these crashes. We believe at least 12 devices have been affected in the last 2 weeks.

This PR updates us to the latest version, which has the fix for the crash.

Steps to reproduce

I wasn't able to reproduce the crash – launch the app and check that it works.

@Ecarrion – I saw that you did a test WatchOS crash a few weeks ago. Can you remember how long it took to show in the Sentry UI?
I also added a fatalError call in the app (not committed, obviously) and checked to see if the log went to Sentry, but it's not showed up yet. I don't imagine it's not working though...

Testing information

Only basic functionality tested.


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

Sentry found a crash in their SDK initialisation process: getsentry/sentry-cocoa#4280

It’s difficult to assess the impact, as we don’t have the benefit of any Sentry issues for these crashes. We believe at least 12 devices have been affected in the last 2 weeks.

This commit updates us to the latest version, which has the fix for the crash.
@joshheald joshheald added category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. type: crash The worst kind of bug. labels Sep 3, 2024
@joshheald joshheald added this to the 20.2 ❄️ milestone Sep 3, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Sep 3, 2024

1 Warning
⚠️ This PR is assigned to the milestone 20.2 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.
1 Message
📖 This PR contains changes to RELEASE-NOTES.txt.
Note that these changes won't affect the final version of the release notes as this version is in code freeze.
Please, get in touch with a release manager if you want to update the final release notes.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr13828-6749038
Version20.2
Bundle IDcom.automattic.alpha.woocommerce
Commit6749038
App Center BuildWooCommerce - Prototype Builds #10693
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@joshheald joshheald requested a review from staskus September 3, 2024 14:35
Copy link
Contributor

@Ecarrion Ecarrion left a comment

Choose a reason for hiding this comment

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

FatalErrors won't be reported to Sentry due to watchOS API limitations! https://wp.me/pe5pgL-5uP

If you want to test out the Sentry integration you can manually log an error!

do {
    try aMethodThatMightFail()
} catch {
    SentrySDK.capture(error: error)
}

@joshheald
Copy link
Contributor Author

Got an error showing with the new SDK (eventually!)

I needed to set UserDefaults.standard.set(true, forKey: CrashLogging.forceCrashLoggingKey) in willFinishLaunchingWithOptions

@joshheald joshheald merged commit 7b09ba9 into release/20.2 Sep 4, 2024
22 checks passed
@joshheald joshheald deleted the issue/13824-update-sentry-sdk-to-crash-free-version branch September 4, 2024 08:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. type: crash The worst kind of bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants