Catch all bundletool exceptions when matching device APKs

directory/src/main/kotlin/app/accrescent/server/directory/ApkMatchingUtils.kt

@@ -8,16 +8,28 @@ import app.accrescent.appstore.v1.DeviceAttributes
 import app.accrescent.bundletool.android.bundle.Commands
 import app.accrescent.bundletool.android.bundle.Devices
 import com.android.tools.build.bundletool.device.ApkMatcher
-import com.android.tools.build.bundletool.model.exceptions.IncompatibleDeviceException
+import com.android.tools.build.bundletool.model.AbiName
+import com.android.tools.build.bundletool.model.exceptions.BundleToolException
 import java.util.Optional
 
+/**
+ * Returns the first ABI in [deviceAttributes]'s device spec that bundletool does not recognize, or
+ * `null` if every supported ABI is known.
+ *
+ * Lets callers reject unrecognized ABIs before matching, since bundletool otherwise surfaces them
+ * inconsistently (an `IncompatibleDeviceException` for split APKs, an `InvalidCommandException` for
+ * multi-ABI ones, or no error at all).
+ */
+fun firstUnrecognizedAbi(deviceAttributes: DeviceAttributes): String? =
+    deviceAttributes.spec.supportedAbisList.firstOrNull { !AbiName.fromPlatformName(it).isPresent }
+
 /**
  * Gets an app's matching APK paths for the given device
  *
  * @param appMetadata the `BuildApksResult` of the app
  * @param deviceAttributes the device attributes of the device
  * @return a list of APK paths in the APK set associated with [appMetadata] and matching the given
- * device, or an empty list if none match
+ * device, or an empty list if none match or bundletool rejects [deviceAttributes]
  */
 fun getMatchingApkPaths(
     appMetadata: Commands.BuildApksResult,
@@ -31,7 +43,11 @@ fun getMatchingApkPaths(
             false,
             true,
         ).getMatchingApks(appMetadata)
-    } catch (_: IncompatibleDeviceException) {
+    } catch (_: BundleToolException) {
+        emptyList()
+    } catch (_: IllegalArgumentException) {
+        // Some matchers (device_tier, country_set, reqGlEsVersion) reject the spec with a plain
+        // IllegalArgumentException rather than a BundleToolException.
         emptyList()
     }.map { it.path.toString() }
 

directory/src/main/kotlin/app/accrescent/server/directory/AppServiceImpl.kt

@@ -5,6 +5,7 @@
 package app.accrescent.server.directory
 
 import app.accrescent.appstore.v1.AppService
+import app.accrescent.appstore.v1.DeviceAttributes
 import app.accrescent.appstore.v1.GetAppDownloadInfoRequest
 import app.accrescent.appstore.v1.GetAppDownloadInfoResponse
 import app.accrescent.appstore.v1.GetAppListingRequest
@@ -223,6 +224,7 @@ class AppServiceImpl @Inject constructor(
         request: GetAppDownloadInfoRequest,
     ): Uni<GetAppDownloadInfoResponse> {
         validateRequestOrThrow(request)
+        validateDeviceAbisOrThrow(request.deviceAttributes)
 
         val response = ReleaseChannel.findByAppIdAndName(
             request.appId,
@@ -275,6 +277,7 @@ class AppServiceImpl @Inject constructor(
     @WithSession
     override fun getAppUpdateInfo(request: GetAppUpdateInfoRequest): Uni<GetAppUpdateInfoResponse> {
         validateRequestOrThrow(request)
+        validateDeviceAbisOrThrow(request.deviceAttributes)
 
         val response = ReleaseChannel.findByAppIdAndName(
             request.appId,
@@ -328,6 +331,20 @@ class AppServiceImpl @Inject constructor(
         return response
     }
 
+    /**
+     * Rejects a request whose device spec contains an ABI bundletool does not recognize.
+     *
+     * @throws StatusRuntimeException if the device spec contains an unrecognized ABI
+     */
+    private fun validateDeviceAbisOrThrow(deviceAttributes: DeviceAttributes) {
+        firstUnrecognizedAbi(deviceAttributes)?.let { abi ->
+            throw Status
+                .fromCode(Status.Code.INVALID_ARGUMENT)
+                .withDescription("unrecognized ABI in device spec: $abi")
+                .asRuntimeException()
+        }
+    }
+
     /**
      * Verifies the request is valid or else throws an exception.
      *

directory/src/test/kotlin/app/accrescent/server/directory/AppServiceImplTest.kt

@@ -271,6 +271,60 @@ class AppServiceImplImplTest {
         )
     }
 
+    @Test
+    fun getAppDownloadInfoWithUnrecognizedDeviceAbiReturnsInvalidArgument() {
+        KafkaHelper.publishApps(kafka, TestDataHelper.validAppPublicationRequested)
+
+        val request = getAppDownloadInfoRequest {
+            appId = "app.accrescent.client"
+            deviceAttributes = deviceAttributesWithUnrecognizedAbi
+        }
+
+        val status = CompletableFuture<Status.Code>()
+        appService.getAppDownloadInfo(request)
+            .subscribe()
+            .with(
+                { status.complete(Status.Code.OK) },
+                {
+                    require(it is StatusRuntimeException)
+                    status.complete(it.status.code)
+                },
+            )
+
+        assertEquals(
+            Status.Code.INVALID_ARGUMENT,
+            status.get(REQUEST_TIMEOUT_SECS, TimeUnit.SECONDS),
+        )
+    }
+
+    @Test
+    fun getAppDownloadInfoWithUnparseableOpenGlVersionReturnsFailedPrecondition() {
+        KafkaHelper.publishApps(kafka, TestDataHelper.validAppPublicationRequested)
+
+        // A non-numeric reqGlEsVersion makes the matcher throw a NumberFormatException
+        // rather than a BundleToolException, exercising the wider backstop catch.
+        val request = getAppDownloadInfoRequest {
+            appId = "app.accrescent.client"
+            deviceAttributes = deviceAttributesWithUnparseableOpenGlVersion
+        }
+
+        val status = CompletableFuture<Status.Code>()
+        appService.getAppDownloadInfo(request)
+            .subscribe()
+            .with(
+                { status.complete(Status.Code.OK) },
+                {
+                    require(it is StatusRuntimeException)
+                    status.complete(it.status.code)
+                },
+            )
+
+        assertEquals(
+            Status.Code.FAILED_PRECONDITION,
+            status.get(REQUEST_TIMEOUT_SECS, TimeUnit.SECONDS),
+        )
+    }
+
     private fun getExpectedAppDownloadInfoResponse() = getAppDownloadInfoResponse {
         appDownloadInfo = appDownloadInfo {
             splitDownloadInfo.addAll(
@@ -307,6 +361,21 @@ class AppServiceImplImplTest {
             }
             .build()
 
+        private val deviceAttributesWithUnrecognizedAbi: DeviceAttributes = validDeviceAttributes
+            .toBuilder()
+            .setSpec(validDeviceAttributes.spec.toBuilder().addSupportedAbis("definitely-not-an-abi"))
+            .build()
+
+        private val deviceAttributesWithUnparseableOpenGlVersion: DeviceAttributes =
+            validDeviceAttributes
+                .toBuilder()
+                .setSpec(
+                    validDeviceAttributes.spec.toBuilder()
+                        .clearDeviceFeatures()
+                        .addDeviceFeatures("reqGlEsVersion=banana")
+                )
+                .build()
+
         private val validGetAppListingRequest = getAppListingRequest {
             appId = "app.accrescent.client"
             preferredLanguages.add("en-US")