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

Race condition when creating and packaging sentry-debug-meta.properties #37

Closed
3 tasks done
headsvk opened this issue Nov 10, 2020 · 18 comments
Closed
3 tasks done
Labels
bug Something isn't working

Comments

@headsvk
Copy link

headsvk commented Nov 10, 2020

Android Gradle Plugin:
4.1.0

Gradle:
6.7

Optimizer:

  • R8

Have you created the sentry.properties file?

  • Yes -> in project root

SDKs:

  • sentry-android -> 3.1.3

I have the following issue:

Android crash reports are not deobfuscated on Sentry.io.
App log shows:

I/Sentry: sentry-debug-meta.properties file was not found.

On clean build, the file is correctly created in build/intermediates/merged_assets folder but it's missing from the final apk.
On rebuild, the file is included in the apk.

Steps to reproduce:

  • Run gradlew clean assembleRelease and open the final apk

Actual result:

  • sentry-debug-meta.properties doesn't get packaged into apk's assets folder

Expected result:

  • sentry-debug-meta.properties gets packaged into apk's assets folder
@mrrobot97
Copy link

I'm also seeing this issue, any workaround?

@mrrobot97
Copy link

I'm having exactly the same issue, if you execute clean task before assembleRelease, the sentry-debug file won't be packaged into the apk, but if you rebuild without a clean task, the sentry-debug file will be packaged into the apk.

AGP version: 4.1.0
gradle version:6.5
sentry-android-gradle-plugin version:1.7.36
sentry-android sdk version:3.1.1

@marandaneto
Copy link
Contributor

that's odd, but I can't reproduce it, already tried a few times, could any of you make a sample case? this could be either because of a combination of Gradle and AGP version or even how the app modules are structured within the project, so a sample case would be helpful, thanks.

@marandaneto marandaneto added the question Further information is requested label Nov 11, 2020
@mrrobot97
Copy link

@marandaneto I upload a demo project and I can reproduce the problem with this empty project. I'm using Android Studio 4.1 btw. Here's the repo: https://github.com/mrrobot97/SentryGradlePluginIssueDemo

@marandaneto marandaneto added bug Something isn't working and removed question Further information is requested labels Nov 11, 2020
@marandaneto
Copy link
Contributor

I can confirm also using AGP 4.1.1 and Gradle 6.7, clean assembleRelease doesn't work sometimes, but running assembleRelease again just works, looking into it

@headsvk
Copy link
Author

headsvk commented Nov 11, 2020

@mrrobot97 great that you could make a repo so quickly!
I want to add I thought it might be a filesystem issue but it happens on my Windows PC and Linux CI

@marandaneto
Copy link
Contributor

marandaneto commented Nov 11, 2020

For a quick update, I can't reproduce the issue if using Gradle <= 6.6.x and AGP <= 4.0.x, so a workaround, for now, would be to downgrade them, still looking into it, I've tested using our latest version btw (1.7.36)

@mrrobot97
Copy link

Yeah I'm using AGP 4.0.2 and it's working correctly.

@marandaneto
Copy link
Contributor

yeah but it's not only about AGP I'd say, Gradle 6.7 and AGP 4.0.x still causes the issue, I need to read their changelogs now and see if I can find out.

@marandaneto
Copy link
Contributor

marandaneto commented Nov 11, 2020

I debugged a bit more and found out the following case on the 2nd run (without the clean):

Task ':app:packageRelease' is not up-to-date because:
Input property 'assets' file .../SentryGradlePluginIssueDemo/app/build/intermediates/merged_assets/release/out/sentry-debug-meta.properties has been added.

we need to execute our task after minifyReleaseWithR8 and before packageRelease, so we have the time to generate the file to be packaged in the final apk, and this is being done at the right time, I can verify this on build scan timeline.

the thing is, looks like Gradle keeps a caching of the files in memory and it's not somehow notified that a new file has been created, in the 2nd run, it identifies a new file (see debug message above) and pack it, I'm still not sure how to solve that, looks like the code is correct but Gradle is not up to date if we write a file on the folder at the last minute, also important to note that we write this file using a 3rd party and not a Gradle task (Copy type).

I'll need to learn a bit more about the internals of Gradle and AGP.

@marandaneto
Copy link
Contributor

I've opened a regression issue on Gradle's repo, sorry about that, looks like a regression on Gradle or AGP rather than a bug on the plugin, unless this is related to the new incremental and caching system which we don't take advantage of it, if it's the case, we'd prioritize to rewrite this plugin as it's on our backlog anyway.

@Whathecode
Copy link

I ran into the same problem using AGP 7.0.0-alpha.03 and Gradle 6.8-rc1.

Running assembleRelease twice is a workaround for now.

@clembou
Copy link

clembou commented Jan 19, 2021

Hi all, I previously hit a very similar issue on a previous version of the plugin, and at the time as a temporary workaround I received the suggestion to force the metadata file generation as described here: getsentry/sentry-java#740 (comment)

Unfortunately no such luck this time, as mentioned above the ordering itself appears to be fine. In our case running the build twice is quite onerous as a workaround, so we will probably roll back for now, if anyone find a way to trick gradle into picking up the merged_assets file before packaging please shout! I have a feeling there should be a way to add a custom gradle task copying the file, but this is beyond my gradle skills.

@bruno-garcia bruno-garcia added this to Backlog in Mobile Platform Team Archived via automation Jan 28, 2021
@bruno-garcia bruno-garcia moved this from Backlog to Next in Mobile Platform Team Archived Jan 28, 2021
@ansman
Copy link
Contributor

ansman commented Feb 12, 2021

Did you consider allowing passing the mapping file ID when uploading them instead of relying on the backend to generate one? This would allow you to work around this issue since you can generate the build ID much earlier in the process.

Of course that's a bigger change since you need to build support in the backend, CLI as well as adding validation.

ansman added a commit to ansman/sentry-android-gradle-plugin that referenced this issue Feb 12, 2021
To ensure that the asset file is added it needs to be available before
the proguard task runs. So instead of relying on the CLI to generate the
UUID there is now a separate task that creates it and --uuid is used in
the CLI to match the mappings to the build.

This fixed getsentry#37
ansman added a commit to ansman/sentry-android-gradle-plugin that referenced this issue Feb 12, 2021
To ensure that the asset file is added it needs to be available before
the proguard task runs. So instead of relying on the CLI to generate the
UUID there is now a separate task that creates it and --uuid is used in
the CLI to match the mappings to the build.

This fixes getsentry#37
ansman added a commit to ansman/sentry-android-gradle-plugin that referenced this issue Feb 12, 2021
To ensure that the asset file is added it needs to be available before
the proguard task runs. So instead of relying on the CLI to generate the
UUID there is now a separate task that creates it and --uuid is used in
the CLI to match the mappings to the build.

This fixes getsentry#37
ansman added a commit to ansman/sentry-android-gradle-plugin that referenced this issue Feb 12, 2021
To ensure that the asset file is added it needs to be available before
the proguard task runs. So instead of relying on the CLI to generate the
UUID there is now a separate task that creates it and --uuid is used in
the CLI to match the mappings to the build.

This fixes getsentry#37
@ansman
Copy link
Contributor

ansman commented Feb 12, 2021

As it turns out the CLI already supports passing the UUID. I've fixed this issue in #41 using the method and I've verified it in our project.

@headsvk
Copy link
Author

headsvk commented Mar 9, 2021

@marandaneto what's the status on merging @ansman's fix? Not having to run assemble task twice would speed up our build pipeline 🙏

@marandaneto
Copy link
Contributor

@headsvk we're currently rewriting the Gradle Plugin and this will solve the issue, using also @ansman fixes but not only.
Subscribe to #50 :)

cortinico pushed a commit to cortinico/sentry-android-gradle-plugin that referenced this issue Mar 12, 2021
To ensure that the asset file is added it needs to be available before
the proguard task runs. So instead of relying on the CLI to generate the
UUID there is now a separate task that creates it and --uuid is used in
the CLI to match the mappings to the build.

This fixes getsentry#37
@bruno-garcia bruno-garcia moved this from Next to Done in Mobile Platform Team Archived Mar 15, 2021
@bruno-garcia bruno-garcia pinned this issue Mar 15, 2021
@marandaneto
Copy link
Contributor

this issue should be solved by https://github.com/getsentry/sentry-android-gradle-plugin/releases/tag/2.0.0-alpha.3
please report any issues you may find, thanks for the patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

6 participants