From af1621025dca6ff458b308b57d70dbd0c0355f9b Mon Sep 17 00:00:00 2001 From: Bethany Date: Mon, 7 Aug 2023 16:26:07 -0700 Subject: [PATCH] wip --- .../__tests__/artifact-http-client.test.ts | 66 ++++++++++--------- .../internal/shared/artifact-twirp-client.ts | 29 ++++---- 2 files changed, 51 insertions(+), 44 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 44469ee6..68495ce2 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -6,20 +6,15 @@ import { createArtifactTwirpClient } from "../src/internal/shared/artifact-twirp import * as core from "@actions/core" -const mockPost = jest.fn((statusCode: number, body: string) => { - const msg = new http.IncomingMessage(new net.Socket()) - msg.statusCode = statusCode - return { - message: msg, - readBody: () => {return Promise.resolve(body)} - } -}) +const mockPost = jest.fn() jest.mock("@actions/http-client", () => { - return jest.fn().mockImplementation(() => { - return { - post: mockPost - } - }) + return { + HttpClient: jest.fn().mockImplementation(() => { + return { + post: mockPost + } + }) + } }) describe("artifact-http-client", () => { @@ -35,7 +30,7 @@ describe("artifact-http-client", () => { beforeEach(() => { mockPost.mockClear(); - (HttpClient as unknown as jest.Mock).mockClear() + }) it("should successfully create a client", () => { @@ -58,7 +53,16 @@ describe("artifact-http-client", () => { } }) */ - const client = createArtifactTwirpClient("upload", new HttpClient()) + mockPost.mockImplementationOnce(() => { + const msg = new http.IncomingMessage(new net.Socket()) + msg.statusCode = 200 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} + } + }) + + const client = createArtifactTwirpClient("upload") const artifact = await client.CreateArtifact( { workflowRunBackendId: "1234", @@ -68,7 +72,7 @@ describe("artifact-http-client", () => { } ) - expect(mockHttpClient).toHaveBeenCalledTimes(1) + expect(mockPost).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() expect(artifact.ok).toBe(true) expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") @@ -92,18 +96,21 @@ describe("artifact-http-client", () => { } }) */ - const msgFailed = new http.IncomingMessage(new net.Socket()) - msgFailed.statusCode = 500 - const msgSucceeded = new http.IncomingMessage(new net.Socket()) - msgSucceeded.statusCode = 200 - const mockPost = jest.fn() - mockPost.mockReturnValueOnce({ - message: msgFailed, - readBody: () => {return Promise.resolve(`{"ok": false}`)} - }).mockReturnValue({ - message: msgSucceeded, - readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} + mockPost.mockImplementationOnce(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 500 + return { + message: msgFailed, + readBody: () => {return Promise.resolve(`{"ok": false}`)} + } + }).mockImplementationOnce(() => { + const msgSucceeded = new http.IncomingMessage(new net.Socket()) + msgSucceeded.statusCode = 200 + return { + message: msgSucceeded, + readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} + } }) /* @@ -124,8 +131,6 @@ describe("artifact-http-client", () => { readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/lol/upload"}`)} } }) - */ - jest.mock("@actions/http-client", () => { return jest.fn().mockImplementation(() => { return { @@ -133,7 +138,6 @@ describe("artifact-http-client", () => { } }) }) - /* jest.mock("@actions/http-client", () => { return { HttpClient: jest.fn().mockImplementation(() => { @@ -152,7 +156,7 @@ describe("artifact-http-client", () => { }) */ - const client = createArtifactTwirpClient("upload", new HttpClient()) + const client = createArtifactTwirpClient("upload") const artifact = await client.CreateArtifact( { workflowRunBackendId: "1234", diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index 7241b0c5..a8e989b0 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -21,17 +21,13 @@ class ArtifactHttpClient implements Rpc { private baseRetryIntervalMilliseconds: number = 3000 private retryMultiplier: number = 1.5 - constructor(userAgent: string, httpClient?: HttpClient) { + constructor(userAgent: string) { const token = getRuntimeToken() this.baseUrl = getResultsServiceUrl() - if (httpClient) { - this.httpClient = httpClient - } else { - this.httpClient = new HttpClient( - userAgent, - [new BearerCredentialHandler(token)], - ) - } + this.httpClient = new HttpClient( + userAgent, + [new BearerCredentialHandler(token)], + ) } // This function satisfies the Rpc interface. It is compatible with the JSON @@ -79,11 +75,18 @@ class ArtifactHttpClient implements Rpc { throw new Error(errorMessage) } + if (attempt + 1 === this.maxAttempts) { + info(`Final attempt failed with error: ${errorMessage}`) + break + } + + const retryTimeMilliseconds = this.getExponentialRetryTimeMilliseconds(attempt) + info( - `Attempt ${attempt + 1} of ${this.maxAttempts} failed with error: ${errorMessage}. Retrying request...` + `Attempt ${attempt + 1} of ${this.maxAttempts} failed with error: ${errorMessage}. Retrying request in ${retryTimeMilliseconds} ms...` ) - await this.sleep(this.getExponentialRetryTimeMilliseconds(attempt)) + await this.sleep(retryTimeMilliseconds) attempt++ } @@ -131,7 +134,7 @@ class ArtifactHttpClient implements Rpc { } } -export function createArtifactTwirpClient(type: "upload" | "download", httpClient?: HttpClient): ArtifactServiceClientJSON { - const client = new ArtifactHttpClient(`@actions/artifact-${type}`, httpClient) +export function createArtifactTwirpClient(type: "upload" | "download"): ArtifactServiceClientJSON { + const client = new ArtifactHttpClient(`@actions/artifact-${type}`) return new ArtifactServiceClientJSON(client) }