From 54e5e8c8866965c5231ba9dfa8585e9cd48c6f73 Mon Sep 17 00:00:00 2001 From: "James M. Greene" Date: Wed, 8 Mar 2023 19:43:11 -0600 Subject: [PATCH] Update error handling for artifact exchange --- src/api-client.js | 75 +++++++++++++++++++++++++++++++++++++++++++---- src/deployment.js | 18 ++++-------- src/index.test.js | 17 +++++++++++ 3 files changed, 93 insertions(+), 17 deletions(-) diff --git a/src/api-client.js b/src/api-client.js index 7b1489e..39479e1 100644 --- a/src/api-client.js +++ b/src/api-client.js @@ -1,10 +1,28 @@ const core = require('@actions/core') const github = require('@actions/github') const hc = require('@actions/http-client') +const { RequestError } = require('@octokit/request-error') +const HttpStatusMessages = require('http-status-messages') // All variables we need from the runtime are loaded here const getContext = require('./context') +// Mostly a lift from https://github.com/octokit/request.js/blob/bd72b7be53ab16a6c1c44be99eb73a328fb1e9e4/src/fetch-wrapper.ts#L151-L165 +// Minor revisions applied. +function toErrorMessage(data) { + if (typeof data === 'string') return data + + if (data != null && 'message' in data) { + if (Array.isArray(data.errors)) { + return `${data.message}: ${data.errors.map(JSON.stringify).join(', ')}` + } + return data.message + } + + // Defer back to the caller + return null +} + async function getSignedArtifactUrl({ runtimeToken, workflowRunId, artifactName }) { const { runTimeUrl: RUNTIME_URL } = getContext() const artifactExchangeUrl = `${RUNTIME_URL}_apis/pipelines/workflows/${workflowRunId}/artifacts?api-version=6.0-preview` @@ -14,11 +32,58 @@ async function getSignedArtifactUrl({ runtimeToken, workflowRunId, artifactName try { core.info(`Artifact exchange URL: ${artifactExchangeUrl}`) - const response = await httpClient.getJson(artifactExchangeUrl, { - Authorization: `Bearer ${runtimeToken}` - }) - - data = response?.result + const requestHeaders = { + accept: 'application/json', + authorization: `Bearer ${runtimeToken}` + } + const res = await httpClient.get(artifactExchangeUrl, requestHeaders) + + // Parse the response body as JSON + let obj = null + try { + const contents = await res.readBody() + if (contents && contents.length > 0) { + obj = JSON.parse(contents) + } + } catch (error) { + // Invalid resource (contents not json); leaving result obj null + } + + // Specific response shape aligned with Octokit + const response = { + url: res.message.url || artifactExchangeUrl, + status: res.message.statusCode || 0, + headers: { + ...res.message?.headers + }, + data: obj + } + + // Forcibly throw errors for negative HTTP status codes! + // @actions/http-client doesn't do this by default. + // Mimic the errors thrown by Octokit for consistency. + if (response.status >= 400) { + throw new RequestError( + toErrorMessage(response.data) || + res.message?.statusMessage || + HttpStatusMessages[response.status] || + 'Unknown error', + response.status, + { + response, + request: { + method: 'GET', + url: artifactExchangeUrl, + headers: { + ...requestHeaders + }, + body: null + } + } + ) + } + + data = response.data core.debug(JSON.stringify(data)) } catch (error) { core.error('Getting signed artifact URL failed', error) diff --git a/src/deployment.js b/src/deployment.js index abef778..6cc8714 100644 --- a/src/deployment.js +++ b/src/deployment.js @@ -80,21 +80,15 @@ class Deployment { // build customized error message based on server response if (error.response) { - let errorMessage = `Failed to create deployment (status: ${error.response.status}) with build version ${this.buildVersion}. ` - if (error.response.status == 400) { - let message = '' - if (error.response.data && error.response.data.message) { - message = error.response.data.message - } else { - message = error.response.data - } - errorMessage += `Responded with: ${message}` - } else if (error.response.status == 403) { + let errorMessage = `Failed to create deployment (status: ${error.status}) with build version ${this.buildVersion}. ` + if (error.status === 400) { + errorMessage += `Responded with: ${error.message}` + } else if (error.status === 403) { errorMessage += 'Ensure GITHUB_TOKEN has permission "pages: write".' - } else if (error.response.status == 404) { + } else if (error.status === 404) { const pagesSettingsUrl = `${this.githubServerUrl}/${this.repositoryNwo}/settings/pages` errorMessage += `Ensure GitHub Pages has been enabled: ${pagesSettingsUrl}` - } else if (error.response.status >= 500) { + } else if (error.status >= 500) { errorMessage += 'Server error, is githubstatus.com reporting a Pages outage? Please re-run the deployment at a later time.' } diff --git a/src/index.test.js b/src/index.test.js index b4a0d6f..5433998 100644 --- a/src/index.test.js +++ b/src/index.test.js @@ -168,6 +168,23 @@ describe('Deployment', () => { createDeploymentScope.done() }) + it('reports errors with failed artifact exchange', async () => { + process.env.GITHUB_SHA = 'invalid-build-version' + const artifactExchangeScope = nock(`http://my-url`) + .get('/_apis/pipelines/workflows/123/artifacts?api-version=6.0-preview') + .reply(400, {}) + + // Create the deployment + const deployment = new Deployment() + await expect(deployment.create()).rejects.toEqual( + new Error( + `Failed to create deployment (status: 400) with build version ${process.env.GITHUB_SHA}. Responded with: Bad Request` + ) + ) + + artifactExchangeScope.done() + }) + it('reports errors with failed deployments', async () => { process.env.GITHUB_SHA = 'invalid-build-version' const artifactExchangeScope = nock(`http://my-url`)