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

NDK symbols upload gradle task should fail if the upload is rejected #529

Open
peterdk opened this issue Jul 9, 2023 · 7 comments
Open
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug

Comments

@peterdk
Copy link

peterdk commented Jul 9, 2023

Description

I recently found out that for some of my apps the NDK so files that got uploaded were not present in the project settings on the website. It turns out that when I manually upload them using curl I get 'invalid file format'. Bugsnag Gradle however happily uploads them and (presumably fails) but doesn't trigger a error. That's not wanted behaviour I would say.

Describe the solution you'd like
If uploading NDK symbol file fails/gets rejected, the gradle task should fail and stop build.

Describe alternatives you've considered
I don't know if the gradle plugin upload call leads to a actual result like the curl method. So maybe this is technically not possible.

Additional context
bugsnag gradle version 8.0.1

@peterdk
Copy link
Author

peterdk commented Jul 9, 2023

It could also be a option in the gradle config, cause it might not suit everyone. Still it's weird that it silently fails.

@johnkiely1
Copy link
Member

Hi @peterdk

By default the plugin should fail the build if the files are not uploaded successfully.

We do have a config that would allow you change the default behaviour by setting it to false.

bugsnag {
    failOnUploadError = false
}

Do you by chance have this enabled?

Did you get anything in the build log to say they weren't uploaded?

Otherwise it would be interesting to see the files that you were attempting to upload that produced the "invalid file format". Assuming they are not too large, would you be able to send them to us [email protected] and we could do some testing of our own.

@johnkiely1 johnkiely1 added the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Jul 10, 2023
@peterdk
Copy link
Author

peterdk commented Jul 10, 2023

Ah weird. It's not explicitly set in my config. I just found out that they result in a error response from the manual api. They are Rust libraries. I already opened a ticket with support for that. Will test with the Gradle task later on tonight and will check response code from manual API.

@peterdk
Copy link
Author

peterdk commented Jul 10, 2023

Ok, I tested this with a custom version of the 8.0.1 branch. (I only added a log message)

 private fun uploadSymbols(mappingFile: File) {
        val sharedObjectName = mappingFile.nameWithoutExtension
        val requestEndpoint = endpoint.get() + ENDPOINT_SUFFIX

        val request = BugsnagMultiPartUploadRequest.from(this, requestEndpoint)
        val manifestInfo = parseManifestInfo()
        val mappingFileHash = mappingFile.md5HashCode()

        val response = uploadRequestClient.get().makeRequestIfNeeded(manifestInfo, mappingFileHash) {
            logger.lifecycle("Bugsnag: Uploading SO mapping file from $mappingFile")
            val body = request.createMultipartBody { builder ->
                builder
                    .addFormDataPart("apiKey", manifestInfo.apiKey)
                    .addFormDataPart("appId", manifestInfo.applicationId)
                    .addFormDataPart("versionCode", manifestInfo.versionCode)
                    .addFormDataPart("versionName", manifestInfo.versionName)
                    .addFormDataPart("soFile", mappingFile.name, mappingFile.asRequestBody())
                    .addFormDataPart("sharedObjectName", sharedObjectName)
                    .addFormDataPart("projectRoot", projectRoot.get())
            }

            request.uploadRequest(body) { response ->
                if (response.code == HTTP_NOT_FOUND && endpoint.get() != UPLOAD_ENDPOINT_DEFAULT) {
                    throw StopExecutionException(
                        "Bugsnag instance does not support the new NDK symbols upload mechanism. " +
                            "Please set legacyNDKSymbolsUpload or upgrade your Bugsnag instance. " +
                            "See https://docs.bugsnag.com/api/ndk-symbol-mapping-upload/ for details."
                    )
                }

                if (!response.isSuccessful) {
                    "Failure"
                } else {
                    response.body!!.string()
                }
            }
        }
        logger.lifecycle("Response: $response") // This is added
        requestOutputFile.asFile.get().writeText(response)
    }

Result:

Task :app:uploadBugsnagNdkReleaseMapping
Bugsnag: Uploading SO mapping file from /[..]/app/build/intermediates/bugsnag/soMappings/ndk/release/x86_64/libdexidizer.so.gz
Response: Failure
Bugsnag: Uploading SO mapping file from /[..]/app/build/intermediates/bugsnag/soMappings/ndk/release/arm64-v8a/libdexidizer.so.gz
Response: Failure
Bugsnag: Uploading SO mapping file from /[..]/app/build/intermediates/bugsnag/soMappings/ndk/release/x86/libdexidizer.so.gz
Response: Failure
Bugsnag: Uploading SO mapping file from //[..]/app/build/intermediates/bugsnag/soMappings/ndk/release/armeabi-v7a/libdexidizer.so.gz
Response: Failure

[..]
BUILD SUCCESSFUL in 58s
76 actionable tasks: 76 executed

So, yes, it detects the failure, but apparently bugsnag gradle doesn't do anything with that. Cause without the added logging I wouldn't know it failed.

@peterdk
Copy link
Author

peterdk commented Jul 10, 2023

I also tried with explicit failOnUploadError=true in the bugsnag settings, but that doesn't do anything.

The change below for example does make it work as intended:

if (!response.isSuccessful) {
    if (failOnUploadError.get())
    {
         throw RuntimeException("Failure when uploading: ${response.body?.string()}")
    }else{
         "Failure"
    }
} else {
    response.body!!.string()
}

So I guess somewhere in the code a exception is not correctly thrown.

@peterdk
Copy link
Author

peterdk commented Jul 11, 2023

Ticket with so file is #55786 in zendesk

@johnkiely1
Copy link
Member

Thanks. we have replied on the ticket.

Mainly for the benefit of future travellers, we are going to look into fixing this behaviour. We will post any updates here.

@johnkiely1 johnkiely1 added bug Confirmed bug backlog We hope to fix this feature/bug in the future and removed awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. labels Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants