Improve exception handling (#507)

* Fatal error on no credentials Improved error UX compared to exception with stack trace * Report invalid path as fatal error * Improve ArgsHelper error & storage auth * Update FtlConstants.kt * Update release_notes.md * Update release_notes.md * Update test suite
Flank · Feb 25, 2019 · 5bcfa87 · 5bcfa87
1 parent b39a08b
commit 5bcfa87
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 14 deletions.
diff --git a/release_notes.md b/release_notes.md
@@ -1,6 +1,7 @@
 ## v?
 
-- Add bugsnag reporting to detect Flank crashes.
+- [#506](https://github.com/TestArmada/flank/pull/506) Add bugsnag reporting to detect Flank crashes. ([bootstraponline](https://github.com/bootstraponline))
+- [#507](https://github.com/TestArmada/flank/pull/507) Improve error message when credentials fail to load, folder doesn't exist, and on bucket creation failure. Properly pass through user credential when checking the storage bucket. ([bootstraponline](https://github.com/bootstraponline))
 
 ## v4.4.0
 

diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsFileVisitor.kt b/test_runner/src/main/kotlin/ftl/args/ArgsFileVisitor.kt
@@ -1,6 +1,8 @@
 package ftl.args
 
+import ftl.util.Utils.fatalError
 import java.io.IOException
+import java.lang.RuntimeException
 import java.nio.file.FileSystems
 import java.nio.file.FileVisitOption
 import java.nio.file.FileVisitResult
@@ -42,7 +44,13 @@ class ArgsFileVisitor(glob: String) : SimpleFileVisitor<Path>() {
         // /Users/tmp/code/*                 => /Users/tmp/code/
         val beforeGlob = Paths.get(searchString.substringBefore(SINGLE_GLOB))
         // must not follow links when resolving paths or /tmp turns into /private/tmp
-        val realPath = beforeGlob.toRealPath(LinkOption.NOFOLLOW_LINKS)
+        val realPath = try {
+            beforeGlob.toRealPath(LinkOption.NOFOLLOW_LINKS)
+        } catch (e: java.nio.file.NoSuchFileException) {
+            fatalError("Failed to resolve path $searchPath")
+            throw RuntimeException("never thrown")
+        }
+
         val searchDepth = if (searchString.contains(RECURSE)) Integer.MAX_VALUE else 1
 
         Files.walkFileTree(realPath, EnumSet.of(FileVisitOption.FOLLOW_LINKS), searchDepth, this)

diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
@@ -22,6 +22,7 @@ import ftl.shard.Shard
 import ftl.shard.StringShards
 import ftl.shard.stringShards
 import ftl.util.Utils
+import ftl.util.Utils.fatalError
 import java.io.File
 import java.net.URI
 import java.nio.file.Files
@@ -111,21 +112,29 @@ object ArgsHelper {
         // because we don't have access to the root account, it won't show up in the storage list.
         if (bucket.startsWith("test-lab-")) return bucket
 
-        val storage = StorageOptions.newBuilder().setProjectId(projectId).build().service
+        val storage = StorageOptions.newBuilder()
+            .setCredentials(FtlConstants.credential)
+            .setProjectId(projectId)
+            .build().service
         val bucketLabel = mapOf("flank" to "")
         val storageLocation = "us-central1"
 
         val bucketListOption = Storage.BucketListOption.prefix(bucket)
         val storageList = storage.list(bucketListOption).values?.map { it.name } ?: emptyList()
         if (storageList.contains(bucket)) return bucket
 
-        return storage.create(
-            BucketInfo.newBuilder(bucket)
-                .setStorageClass(StorageClass.REGIONAL)
-                .setLocation(storageLocation)
-                .setLabels(bucketLabel)
-                .build()
-        ).name
+        try {
+            return storage.create(
+                BucketInfo.newBuilder(bucket)
+                    .setStorageClass(StorageClass.REGIONAL)
+                    .setLocation(storageLocation)
+                    .setLabels(bucketLabel)
+                    .build()
+            ).name
+        } catch (e: com.google.cloud.storage.StorageException) {
+            fatalError("Failed to create bucket $bucket\n${e.message}")
+            throw RuntimeException("will not throw")
+        }
     }
 
     private fun serviceAccountProjectId(): String? {

diff --git a/test_runner/src/main/kotlin/ftl/config/FtlConstants.kt b/test_runner/src/main/kotlin/ftl/config/FtlConstants.kt
@@ -5,6 +5,7 @@ import ch.qos.logback.classic.Logger
 import com.bugsnag.Bugsnag
 import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport
 import com.google.api.client.googleapis.util.Utils
+import com.google.api.client.http.GoogleApiLogger
 import com.google.api.client.http.HttpRequestInitializer
 import com.google.api.client.http.javanet.NetHttpTransport
 import com.google.api.client.json.JsonFactory
@@ -16,7 +17,9 @@ import ftl.args.IArgs
 import ftl.args.IosArgs
 import ftl.gc.UserAuth
 import ftl.http.HttpTimeoutIncrease
+import ftl.util.Utils.fatalError
 import ftl.util.Utils.readRevision
+import java.io.IOException
 import java.nio.file.Path
 import java.nio.file.Paths
 import java.util.Date
@@ -64,7 +67,13 @@ object FtlConstants {
         when {
             useMock -> GoogleCredentials.create(AccessToken("mock", Date()))
             UserAuth.exists() -> UserAuth.load()
-            else -> ServiceAccountCredentials.getApplicationDefault()
+            else -> try {
+                GoogleApiLogger.silenceComputeEngine()
+                ServiceAccountCredentials.getApplicationDefault()
+            } catch (e: IOException) {
+                fatalError("Error: Failed to read service account credential.\n${e.message}")
+                throw RuntimeException("never thrown")
+            }
         }
     }
 

diff --git a/test_runner/src/test/kotlin/ftl/args/ArgsHelperFilePathTest.kt b/test_runner/src/test/kotlin/ftl/args/ArgsHelperFilePathTest.kt
@@ -124,13 +124,13 @@ class ArgsHelperFilePathTest {
         Truth.assertThat(actual).isEqualTo(expected)
     }
 
-    @Test(expected = java.nio.file.NoSuchFileException::class)
+    @Test(expected = RuntimeException::class)
     fun evaluateInvalidFilePath() {
         val testApkPath = "~/flank_test_app/invalid_path/app-debug-*.xctestrun"
         ArgsHelper.evaluateFilePath(testApkPath)
     }
 
-    @Test(expected = java.nio.file.NoSuchFileException::class)
+    @Test(expected = RuntimeException::class)
     fun evaluateInvalidFilePathWithTilde() {
         val testApkPath = "~/flank_test_app/~/invalid_path/app-debug-*.xctestrun"
         ArgsHelper.evaluateFilePath(testApkPath)

diff --git a/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt b/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt
@@ -161,7 +161,7 @@ class ArgsHelperTest {
         assertThat(actual).isEqualTo(expected)
     }
 
-    @Test(expected = java.nio.file.NoSuchFileException::class)
+    @Test(expected = RuntimeException::class)
     fun evaluateInvalidFilePath() {
         val testApkPath = "~/flank_test_app/invalid_path/app-debug-*.xctestrun"
         ArgsHelper.evaluateFilePath(testApkPath)