fix(Dangerfile): report errors when evaluating Dangerfile

resolves #250

changelog.md

@@ -4,6 +4,8 @@
 
 // TODO
 
+* Handle exceptions in Dangerfile and report them as failures in Danger results - macklinu
+
 ### 2.0.0-alpha.6-7
 
 * Expose a Promise object to the external GitHub API - orta

source/runner/DangerfileRunner.ts

@@ -92,26 +92,36 @@ export async function runDangerfileEnvironment(
   //   return originalRequire.apply(this, arguments)
   // }
 
-  vm.run(content, filename)
-
-  const results = environment.sandbox!.results!
-  await Promise.all(
-    results.scheduled.map((fnOrPromise: any) => {
-      if (fnOrPromise instanceof Promise) {
-        return fnOrPromise
-      }
-      if (fnOrPromise.length === 1) {
-        // callback-based function
-        return new Promise(res => fnOrPromise(res))
-      }
-      return fnOrPromise()
-    })
-  )
-  return {
-    fails: results.fails,
-    warnings: results.warnings,
-    messages: results.messages,
-    markdowns: results.markdowns,
+  try {
+    vm.run(content, filename)
+
+    const results = environment.sandbox!.results!
+    await Promise.all(
+      results.scheduled.map((fnOrPromise: any) => {
+        if (fnOrPromise instanceof Promise) {
+          return fnOrPromise
+        }
+        if (fnOrPromise.length === 1) {
+          // callback-based function
+          return new Promise(res => fnOrPromise(res))
+        }
+        return fnOrPromise()
+      })
+    )
+    return {
+      fails: results.fails,
+      warnings: results.warnings,
+      messages: results.messages,
+      markdowns: results.markdowns,
+    }
+  } catch (e) {
+    console.error("Unable to evaluate the Dangerfile")
+    return {
+      fails: [{ message: `\`\`\`\n${e.stack}\n\`\`\`` }],
+      warnings: [],
+      messages: [],
+      markdowns: [],
+    }
   }
 }
 

source/runner/_tests/_danger_runner.test.ts

@@ -61,14 +61,9 @@ describe("with fixtures", () => {
   it("handles a failing dangerfile", async () => {
     const context = await setupDangerfileContext()
     const runtime = await createDangerfileRuntimeEnvironment(context)
+    const results = await runDangerfileEnvironment(resolve(fixtures, "__DangerfileBadSyntax.js"), undefined, runtime)
 
-    try {
-      await runDangerfileEnvironment(resolve(fixtures, "__DangerfileBadSyntax.js"), undefined, runtime)
-      throw new Error("Do not get to this")
-    } catch (e) {
-      // expect(e.message === ("Do not get to this")).toBeFalsy()
-      expect(e.message).toEqual("hello is not defined")
-    }
+    expect(results.fails[0].message).toContain("hello is not defined")
   })
 
   it.skip("handles relative imports correctly in Babel", async () => {
@@ -178,6 +173,14 @@ describe("with fixtures", () => {
 
     expect(results.fails[0].message).toContain("@types dependencies were added to package.json")
   })
+
+  it("does not swallow errors thrown in Dangerfile", async () => {
+    const context = await setupDangerfileContext()
+    const runtime = await createDangerfileRuntimeEnvironment(context)
+    const results = await runDangerfileEnvironment(resolve(fixtures, "__DangerfileThrows.js"), undefined, runtime)
+
+    expect(results.fails[0].message).toContain("Error: failure")
+  })
 })
 
 describe("cleaning Dangerfiles", () => {

source/runner/_tests/fixtures/__DangerfileThrows.js

@@ -0,0 +1 @@
+throw new Error("failure")