Merge pull request #32 from dependabot/brrygrdn/handle-failure-during…

…-fetch

Avoid double-reporting errors when fetching files

__tests__/container-service.test.ts

@@ -36,7 +36,9 @@ describe('ContainerService', () => {
     })
 
     test('raises an exception', async () => {
-      await expect(ContainerService.run(container)).rejects.toThrow()
+      await expect(ContainerService.run(container)).rejects.toThrow(
+        /Failure running container/
+      )
     })
   })
 })

__tests__/main.test.ts

@@ -3,7 +3,7 @@ import path from 'path'
 import * as core from '@actions/core'
 import {Context} from '@actions/github/lib/context'
 import {ApiClient} from '../src/api-client'
-import {Updater} from '../src/updater'
+import {Updater, UpdaterFetchError} from '../src/updater'
 import {ImageService} from '../src/image-service'
 import * as inputs from '../src/inputs'
 import {run} from '../src/main'
@@ -272,4 +272,39 @@ describe('run', () => {
       expect(markJobAsProcessedSpy).toHaveBeenCalled()
     })
   })
+
+  describe('when the file fetch step fails', () => {
+    beforeEach(() => {
+      jest
+        .spyOn(Updater.prototype, 'runUpdater')
+        .mockImplementationOnce(
+          jest.fn(async () =>
+            Promise.reject(
+              new UpdaterFetchError(
+                'No output.json created by the fetcher container'
+              )
+            )
+          )
+        )
+
+      context = new Context()
+    })
+
+    test('it fails the workflow', async () => {
+      await run(context)
+
+      expect(core.setFailed).toHaveBeenCalledWith(
+        expect.stringContaining(
+          'Dependabot was unable to retrieve the files required to perform the update'
+        )
+      )
+    })
+
+    test('it does not inform dependabot-api as the failed fetch step will have already reported in', async () => {
+      await run(context)
+
+      expect(markJobAsProcessedSpy).not.toHaveBeenCalled()
+      expect(reportJobErrorSpy).not.toHaveBeenCalled()
+    })
+  })
 })

__tests__/updater.test.ts

@@ -1,6 +1,6 @@
 import fs from 'fs'
 import path from 'path'
-import {Updater} from '../src/updater'
+import {Updater, UpdaterFetchError} from '../src/updater'
 import Docker from 'dockerode'
 import {ContainerService} from '../src/container-service'
 import {ProxyBuilder} from '../src/proxy'
@@ -212,9 +212,9 @@ describe('Updater', () => {
       jest.spyOn(ContainerService, 'run').mockImplementation(jest.fn())
     })
 
-    it('should raise an error', async () => {
+    it('should raise a UpdaterFetchError', async () => {
       await expect(updater.runUpdater()).rejects.toThrow(
-        new Error('No output.json created by the fetcher container')
+        new UpdaterFetchError('No output.json created by the fetcher container')
       )
     })
   })

src/main.ts

@@ -3,7 +3,7 @@ import * as github from '@actions/github'
 import {Context} from '@actions/github/lib/context'
 import {getJobParameters} from './inputs'
 import {ImageService} from './image-service'
-import {Updater} from './updater'
+import {Updater, UpdaterFetchError} from './updater'
 import {ApiClient} from './api-client'
 import axios from 'axios'
 
@@ -56,9 +56,8 @@ export async function run(context: Context): Promise<void> {
         params.workingDirectory
       )
 
+      core.startGroup('Pulling updater images')
       try {
-        core.info('Pulling updater images')
-
         await ImageService.pull(UPDATER_IMAGE_NAME)
         await ImageService.pull(PROXY_IMAGE_NAME)
       } catch (error) {
@@ -67,15 +66,27 @@ export async function run(context: Context): Promise<void> {
         await failJob(apiClient, error, DependabotErrorType.Image)
         return
       }
+      core.endGroup()
 
       try {
         core.info('Starting update process')
 
         await updater.runUpdater()
       } catch (error) {
-        core.error('Error performing update')
-        await failJob(apiClient, error, DependabotErrorType.UpdateRun)
-        return
+        // If we have encountered a UpdaterFetchError, the Updater will already have
+        // reported the error and marked the job as processed, so we only need to
+        // set an exit status.
+        if (error instanceof UpdaterFetchError) {
+          core.setFailed(
+            'Dependabot was unable to retrieve the files required to perform the update'
+          )
+          core.info('🤖 ~ finished: unable to fetch files ~')
+          return
+        } else {
+          core.error('Error performing update')
+          await failJob(apiClient, error, DependabotErrorType.UpdateRun)
+          return
+        }
       }
       core.info('🤖 ~ finished ~')
     } catch (error) {

src/updater.ts

@@ -16,6 +16,13 @@ const REPO_CONTENTS_PATH = '/home/dependabot/dependabot-updater/repo'
 const CA_CERT_INPUT_PATH = '/usr/local/share/ca-certificates'
 const CA_CERT_FILENAME = 'dbot-ca.crt'
 
+export class UpdaterFetchError extends Error {
+  constructor(msg: string) {
+    super(msg)
+    Object.setPrototypeOf(this, UpdaterFetchError.prototype)
+  }
+}
+
 export class Updater {
   docker: Docker
   outputHostPath: string
@@ -77,7 +84,9 @@ export class Updater {
 
     const outputPath = path.join(this.outputHostPath, 'output.json')
     if (!fs.existsSync(outputPath)) {
-      throw new Error('No output.json created by the fetcher container')
+      throw new UpdaterFetchError(
+        'No output.json created by the fetcher container'
+      )
     }
 
     const fileFetcherSync = fs.readFileSync(outputPath).toString()