-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
@@ -304,7 +304,7 @@ internal void SetupSymbolsUpload(string unityProjectPath, string gradleProjectPa | |||
{ | |||
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.
@@ -102,8 +103,6 @@ public DebugSymbolUpload(IDiagnosticLogger logger, | |||
|
|||
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 <[email protected]>
Co-authored-by: stefan.jandl <[email protected]>
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.
.properties
regex.Replace
not doing what we expect it to do?