From 16b786a545a0b3a90e4dc4330af7225cf06f7e93 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Mon, 11 Dec 2023 22:01:08 -0500 Subject: [PATCH] better error message for usage limits --- .../__tests__/artifact-http-client.test.ts | 40 +++++++++++++++++-- .../internal/shared/artifact-twirp-client.ts | 35 +++++++++++----- .../artifact/src/internal/shared/errors.ts | 13 ++++++ 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index d96c0d59..e6768346 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -4,6 +4,7 @@ import {HttpClient} from '@actions/http-client' import * as config from '../src/internal/shared/config' import {internalArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client' import {noopLogs} from './common' +import {NetworkError, UsageError} from '../src/internal/shared/errors' jest.mock('@actions/http-client') @@ -257,9 +258,42 @@ describe('artifact-http-client', () => { name: 'artifact', version: 4 }) - }).rejects.toThrowError( - 'Failed to CreateArtifact: Unable to make request: ENOTFOUND\nIf you are using self-hosted runners, please make sure your runner has access to all GitHub endpoints: https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#communication-between-self-hosted-runners-and-github' - ) + }).rejects.toThrowError(new NetworkError('ENOTFOUND').message) + expect(mockHttpClient).toHaveBeenCalledTimes(1) + expect(mockPost).toHaveBeenCalledTimes(1) + }) + + it('should properly describe a usage error', async () => { + const mockPost = jest.fn(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 403 + msgFailed.statusMessage = 'Forbidden' + return { + message: msgFailed, + readBody: async () => { + return Promise.resolve( + `{"msg": "insufficient usage to create artifact"}` + ) + } + } + }) + + const mockHttpClient = ( + HttpClient as unknown as jest.Mock + ).mockImplementation(() => { + return { + post: mockPost + } + }) + const client = internalArtifactTwirpClient() + await expect(async () => { + await client.CreateArtifact({ + workflowRunBackendId: '1234', + workflowJobRunBackendId: '5678', + name: 'artifact', + version: 4 + }) + }).rejects.toThrowError(new UsageError().message) expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(mockPost).toHaveBeenCalledTimes(1) }) diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index be26c648..1f987a1a 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -4,7 +4,7 @@ import {info, debug} from '@actions/core' import {ArtifactServiceClientJSON} from '../../generated' import {getResultsServiceUrl, getRuntimeToken} from './config' import {getUserAgentString} from './user-agent' -import {NetworkError} from './errors' +import {NetworkError, UsageError} from './errors' // The twirp http client must implement this interface interface Rpc { @@ -64,7 +64,7 @@ class ArtifactHttpClient implements Rpc { this.httpClient.post(url, JSON.stringify(data), headers) ) - return JSON.parse(body) + return body } catch (error) { throw new Error(`Failed to ${method}: ${error.message}`) } @@ -72,34 +72,49 @@ class ArtifactHttpClient implements Rpc { async retryableRequest( operation: () => Promise - ): Promise<{response: HttpClientResponse; body: string}> { + ): Promise<{response: HttpClientResponse; body: object}> { let attempt = 0 let errorMessage = '' + let rawBody = '' while (attempt < this.maxAttempts) { let isRetryable = false try { const response = await operation() const statusCode = response.message.statusCode - const body = await response.readBody() + rawBody = await response.readBody() debug(`[Response] - ${response.message.statusCode}`) debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`) - debug(`Body: ${body}`) + const body = JSON.parse(rawBody) + debug(`Body: ${JSON.stringify(body, null, 2)}`) if (this.isSuccessStatusCode(statusCode)) { return {response, body} } isRetryable = this.isRetryableHttpStatusCode(statusCode) errorMessage = `Failed request: (${statusCode}) ${response.message.statusMessage}` - const responseMessage = JSON.parse(body).msg - if (responseMessage) { - errorMessage = `${errorMessage}: ${responseMessage}` + if (body.msg) { + if (UsageError.isUsageErrorMessage(body.msg)) { + throw new UsageError() + } + + errorMessage = `${errorMessage}: ${body.msg}` } } catch (error) { - isRetryable = true - errorMessage = error.message + if (error instanceof SyntaxError) { + debug(`Raw Body: ${rawBody}`) + throw error + } + + if (error instanceof UsageError) { + throw error + } + if (NetworkError.isNetworkErrorCode(error?.code)) { throw new NetworkError(error?.code) } + + isRetryable = true + errorMessage = error.message } if (!isRetryable) { diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts index 4f23ecd3..1ceb5c66 100644 --- a/packages/artifact/src/internal/shared/errors.ts +++ b/packages/artifact/src/internal/shared/errors.ts @@ -57,3 +57,16 @@ export class NetworkError extends Error { ].includes(code) } } + +export class UsageError extends Error { + constructor() { + const message = `Artifact storage quota has been hit. Unable to upload any new artifacts. Usage is recalculated every 6-12 hours.\nMore info on storage limits: https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#calculating-minute-and-storage-spending` + super(message) + this.name = 'UsageError' + } + + static isUsageErrorMessage = (msg?: string): boolean => { + if (!msg) return false + return msg.includes('insufficient usage') + } +}