-
-
Notifications
You must be signed in to change notification settings - Fork 58
fix: Prevent appending uploadTask more than once
#2300
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
Conversation
| symbolsUpload.RemoveUploadFromGradleFile(); | ||
|
|
||
| if (_options is { Il2CppLineNumberSupportEnabled: true }) | ||
| if (_options is { Enabled: true, Il2CppLineNumberSupportEnabled: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated but: To reduce logging noise when the SDK is disabled.
|
|
||
| public void AppendUploadToGradleFile(string sentryCliPath) | ||
| { | ||
| RemoveUploadFromGradleFile(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now happens outside and before calling this method. Added a sanity check before actually writing to disk.
…y/sentry-unity into fix/android-upload-task
|
@sentry review |
|
🔒 GenAI Consent Required To enable PR review and test generation via Prevent, an organization admin needs to:
Once enabled, you can re-trigger this review by commenting. |
|
bugbot review |
… block Co-authored-by: stefan.jandl <stefan.jandl@sentry.io>
Co-authored-by: stefan.jandl <stefan.jandl@sentry.io>
Addressed the concern raised in regards to the regex being off.
Fixes #2195, #2170
Problem
I still fail to reproduce this on both macOS and Windows. But given that the issue is appending the upload task multiple times there are only two places where this could fail:
sentry-unity/src/Sentry.Unity.Editor/Android/DebugSymbolUpload.cs
Line 185 in dbbb63c
sentry-unity/src/Sentry.Unity.Editor/Android/DebugSymbolUpload.cs
Lines 191 to 192 in dbbb63c
Proposal
The SDK is needs to be more defensive about adding the upload task to the Gradle file and prevent duplicated appending.
.propertiesregex.Replacenot doing what we expect it to do?