Merge branch 'master' of https://github.com/danger/danger-js into use…

…_diff

changelog.md

@@ -4,9 +4,10 @@
 
 // TODO
 
-### 2.0.0-alpha.7
+### 2.0.0-alpha.8
 
 * Uses the GitHub `diff_url` instead of the `diff` version header, as it conflicted with Peril - orta
+* Handle exceptions in Dangerfile and report them as failures in Danger results - macklinu
 
 ### 2.0.0-alpha.6-7
 

package.json

@@ -93,6 +93,7 @@
     "node-fetch": "^1.6.3",
     "parse-diff": "^0.4.0",
     "parse-link-header": "^1.0.1",
+    "pinpoint": "^1.1.0",
     "rfc6902": "^1.3.0",
     "vm2": "patriksimek/vm2",
     "voca": "^1.2.0"

source/ambient.d.ts

@@ -11,6 +11,7 @@ declare module "voca"
 declare module "jsome"
 declare module "jsonpointer"
 declare module "parse-link-header"
+declare module "pinpoint"
 
 declare module "*/package.json"
 

source/runner/DangerfileRunner.ts

@@ -1,5 +1,6 @@
 import * as fs from "fs"
 import * as path from "path"
+import * as pinpoint from "pinpoint"
 
 import { DangerResults } from "../dsl/DangerResults"
 import { DangerContext } from "../runner/Dangerfile"
@@ -92,27 +93,56 @@ 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 (error) {
+    console.error("Unable to evaluate the Dangerfile")
+    return resultsForCaughtError(filename, content, error)
+  }
+}
+
+/** Returns Markdown results to post if an exception is raised during the danger run */
+const resultsForCaughtError = (file: string, contents: string, error: Error): DangerResults => {
+  const match = /(\d+:\d+)/g.exec(error.stack!)
+  let code
+  if (match) {
+    const [line, column] = match[0].split(":").map(value => parseInt(value, 10) - 1)
+    code = pinpoint(contents, { line, column })
+  } else {
+    code = contents
   }
+  const failure = `Danger failed to run \`${file}\`.`
+  const errorMD = `## Error ${error.name}
+\`\`\`
+${error.message}
+${error.stack}
+\`\`\`
+### Dangerfile
+\`\`\`
+${code}
+\`\`\`
+  `
+  return { fails: [{ message: failure }], warnings: [], markdowns: [errorMD], messages: [] }
 }
 
 // https://regex101.com/r/dUq4yB/1

source/runner/_tests/_danger_runner.test.ts

@@ -61,14 +61,10 @@ 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("Danger failed to run")
+    expect(results.markdowns[0]).toContain("hello is not defined")
   })
 
   it.skip("handles relative imports correctly in Babel", async () => {
@@ -178,6 +174,15 @@ 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("Danger failed to run")
+    expect(results.markdowns[0]).toContain("Error: failure")
+  })
 })
 
 describe("cleaning Dangerfiles", () => {

source/runner/_tests/fixtures/__DangerfileThrows.js

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

yarn.lock

@@ -3328,6 +3328,10 @@ pinkie@^2.0.0:
   version "2.0.4"
   resolved "https://registry.yarnpkg.com/pinkie/-/pinkie-2.0.4.tgz#72556b80cfa0d48a974e80e77248e80ed4f7f870"
 
+pinpoint@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/pinpoint/-/pinpoint-1.1.0.tgz#0cf7757a6977f1bf7f6a32207b709e377388e874"
+
 pkg-dir@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/pkg-dir/-/pkg-dir-2.0.0.tgz#f6d5d1109e19d63edf428e0bd57e12777615334b"