Skip to content

Commit c7daa5d

Browse files
lfkellogglfkellogg
authored
Show user localized error messages and remove special 404 handling (#8266)
Previously we'd assumed any 404 when calling CreateReleaseTest meant the test case did not exist. Now, 404s can also mean that the results bucket does not exist. This PR handles this better by simply relaying the best available message from the RPC response.
1 parent 35a4f1b commit c7daa5d

5 files changed

Lines changed: 114 additions & 37 deletions

File tree

‎firebase-appdistribution-gradle/src/main/java/com/google/firebase/appdistribution/gradle/ApiService.kt‎

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package com.google.firebase.appdistribution.gradle
1919
import com.google.api.client.http.ByteArrayContent
2020
import com.google.api.client.http.HttpResponseException
2121
import com.google.firebase.appdistribution.gradle.AppDistributionException.Reason.Companion.processingBinaryError
22-
import com.google.firebase.appdistribution.gradle.AppDistributionException.Reason.TEST_CASE_NOT_FOUND
2322
import com.google.firebase.appdistribution.gradle.AppDistributionException.Reason.TOO_MANY_TESTER_EMAILS
2423
import com.google.firebase.appdistribution.gradle.NameUtils.extractResourceId
2524
import com.google.firebase.appdistribution.gradle.models.AabInfo
@@ -107,35 +106,27 @@ class ApiService(private val httpClient: AuthenticatedHttpClient) {
107106
testCase = testCaseName,
108107
resultsBucket = resultsBucketName
109108
)
110-
try {
111-
val response =
112-
httpClient
113-
.newPostRequest(
114-
ApiEndpoints.getCreateReleaseTestEndpoint(releaseName),
115-
buildHttpContent(Gson().toJsonTree(releaseTest))
116-
)
117-
.execute()
118-
119-
return if (response.isSuccessStatusCode) {
120-
val prefix =
121-
if (testCaseName != null) "Started test case ${extractResourceId(testCaseName)}"
122-
else "Started test"
123-
logger.lifecycle(
124-
"{} successfully [{}]. Note: This feature is in beta.",
125-
prefix,
126-
response.statusCode
109+
val response =
110+
httpClient
111+
.newPostRequest(
112+
ApiEndpoints.getCreateReleaseTestEndpoint(releaseName),
113+
buildHttpContent(Gson().toJsonTree(releaseTest))
127114
)
128-
Gson().fromJson(response.parseAsString(), ReleaseTest::class.java)
129-
} else {
130-
logger.warn("Unable to start test. Response code: {}", response.statusCode)
131-
null
132-
}
133-
} catch (e: HttpResponseException) {
134-
if (e.statusCode == 404) {
135-
throw AppDistributionException(TEST_CASE_NOT_FOUND, extraInformation = testCaseName)
136-
}
115+
.execute()
137116

138-
throw e
117+
return if (response.isSuccessStatusCode) {
118+
val prefix =
119+
if (testCaseName != null) "Started test case ${extractResourceId(testCaseName)}"
120+
else "Started test"
121+
logger.lifecycle(
122+
"{} successfully [{}]. Note: This feature is in beta.",
123+
prefix,
124+
response.statusCode
125+
)
126+
Gson().fromJson(response.parseAsString(), ReleaseTest::class.java)
127+
} else {
128+
logger.warn("Unable to start test. Response code: {}", response.statusCode)
129+
null
139130
}
140131
}
141132

‎firebase-appdistribution-gradle/src/main/java/com/google/firebase/appdistribution/gradle/AppDistributionException.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import com.google.api.client.http.HttpResponseException
2020
import com.google.firebase.appdistribution.gradle.models.AabState
2121
import com.google.firebase.appdistribution.gradle.models.WrappedErrorResponse
2222
import com.google.gson.Gson
23-
import com.google.gson.JsonSyntaxException
2423
import java.io.IOException
2524
import org.gradle.api.logging.Logging
2625

@@ -136,10 +135,13 @@ constructor(
136135
var message = exception.statusMessage
137136
try {
138137
val response = Gson().fromJson(exception.content, WrappedErrorResponse::class.java)
139-
if (response?.error != null) {
140-
message = response.error.message
138+
val error = response?.error
139+
if (error != null) {
140+
val localizedMessageDetail =
141+
error.details?.find { it.type == "type.googleapis.com/google.rpc.LocalizedMessage" }
142+
message = localizedMessageDetail?.message ?: error.message ?: exception.statusMessage
141143
}
142-
} catch (e: JsonSyntaxException) {
144+
} catch (e: Exception) {
143145
logger.warn("Failed to parse error response: {}", exception.content)
144146
}
145147
return message

‎firebase-appdistribution-gradle/src/main/java/com/google/firebase/appdistribution/gradle/models/ErrorPayload.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,11 @@ data class ErrorPayload(
3131
@SerializedName("message") val message: String? = null,
3232
@SerializedName("code") val code: Int = 0,
3333
@SerializedName("status") val status: String? = null,
34+
@SerializedName("details") val details: List<ErrorDetail>? = null,
35+
)
36+
37+
data class ErrorDetail(
38+
@SerializedName("@type") val type: String? = null,
39+
@SerializedName("message") val message: String? = null,
40+
@SerializedName("locale") val locale: String? = null,
3441
)

‎firebase-appdistribution-gradle/src/test/java/com/google/firebase/appdistribution/gradle/ApiServiceTest.kt‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616

1717
package com.google.firebase.appdistribution.gradle
1818

19+
import com.google.api.client.http.HttpResponseException
1920
import com.google.common.collect.ImmutableList
2021
import com.google.common.collect.Lists
21-
import com.google.firebase.appdistribution.gradle.AppDistributionException.Reason.TEST_CASE_NOT_FOUND
2222
import com.google.firebase.appdistribution.gradle.AppDistributionException.Reason.TOO_MANY_TESTER_EMAILS
2323
import com.google.firebase.appdistribution.gradle.models.AabState
2424
import com.google.firebase.appdistribution.gradle.models.DeviceExecution
@@ -206,10 +206,7 @@ class ApiServiceTest {
206206
val httpTransport = AppDistroMockHttpTransport.newBuilder().setCode(404).build()
207207
val httpClient = AuthenticatedHttpClient(httpTransport)
208208
val apiService = ApiService(httpClient)
209-
assertFailsWith(
210-
AppDistributionException::class,
211-
AppDistributionException.formatMessage(TEST_CASE_NOT_FOUND, "invalid-test-case-id"),
212-
) {
209+
assertFailsWith(HttpResponseException::class) {
213210
apiService.testRelease(
214211
RELEASE_NAME,
215212
listOf(TestDevice(model = "pixel")),

‎firebase-appdistribution-gradle/src/test/java/com/google/firebase/appdistribution/gradle/AppDistributionExceptionTest.kt‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.firebase.appdistribution.gradle
18+
19+
import com.google.api.client.http.HttpHeaders
20+
import com.google.api.client.http.HttpResponseException
21+
import kotlin.test.assertContains
22+
import kotlin.test.assertEquals
23+
import org.junit.Test
24+
25+
class AppDistributionExceptionTest {
26+
27+
@Test
28+
fun fromHttpResponseException_withMessageField_parsesCorrectly() {
29+
val errorResponse =
30+
"{\"error\":{\"code\":404,\"message\":\"The GCS bucket 'my-bucket' was not found\",\"status\":\"NOT_FOUND\"}}"
31+
val httpResponseException = createHttpResponseException(404, errorResponse)
32+
33+
val exception =
34+
AppDistributionException.fromHttpResponseException(
35+
AppDistributionException.Reason.STARTING_TEST_FAILED,
36+
httpResponseException
37+
)
38+
39+
assertEquals(AppDistributionException.Reason.STARTING_TEST_FAILED, exception.reason)
40+
assertContains(exception.message!!, "The GCS bucket 'my-bucket' was not found")
41+
}
42+
43+
@Test
44+
fun fromHttpResponseException_withLocalizedMessageDetail_parsesCorrectly() {
45+
val errorResponse =
46+
"{\"error\":{\"code\":404,\"message\":\"Resource not found\",\"status\":\"NOT_FOUND\"," +
47+
"\"details\":[{\"@type\":\"type.googleapis.com/google.rpc.LocalizedMessage\",\"message\":\"User-facing custom error message\"}]}}"
48+
val httpResponseException = createHttpResponseException(404, errorResponse)
49+
50+
val exception =
51+
AppDistributionException.fromHttpResponseException(
52+
AppDistributionException.Reason.STARTING_TEST_FAILED,
53+
httpResponseException
54+
)
55+
56+
assertEquals(AppDistributionException.Reason.STARTING_TEST_FAILED, exception.reason)
57+
assertContains(exception.message!!, "User-facing custom error message")
58+
}
59+
60+
@Test
61+
fun fromHttpResponseException_withGenericResponse_fallsBackToStatusMessage() {
62+
val httpResponseException = createHttpResponseException(404, "")
63+
64+
val exception =
65+
AppDistributionException.fromHttpResponseException(
66+
AppDistributionException.Reason.STARTING_TEST_FAILED,
67+
httpResponseException
68+
)
69+
70+
assertEquals(AppDistributionException.Reason.STARTING_TEST_FAILED, exception.reason)
71+
assertContains(exception.message!!, "Not Found")
72+
}
73+
74+
private fun createHttpResponseException(statusCode: Int, content: String): HttpResponseException {
75+
val statusMessage = if (statusCode == 404) "Not Found" else "OK"
76+
return HttpResponseException.Builder(statusCode, statusMessage, HttpHeaders())
77+
.setContent(content)
78+
.build()
79+
}
80+
}

0 commit comments

Comments
 (0)