From 8a5343d54ae166977fcba7a41ccc7a43861cb9f3 Mon Sep 17 00:00:00 2001 From: Bethany Date: Fri, 4 Aug 2023 09:23:14 -0700 Subject: [PATCH 01/15] add twirp client --- .../internal/shared/artifact-twirp-client.ts | 133 ++++++++++++++++++ .../artifact/src/internal/shared/config.ts | 15 ++ 2 files changed, 148 insertions(+) create mode 100644 packages/artifact/src/internal/shared/artifact-twirp-client.ts create mode 100644 packages/artifact/src/internal/shared/config.ts diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts new file mode 100644 index 00000000..02ac36c3 --- /dev/null +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -0,0 +1,133 @@ +import { HttpClient, HttpClientResponse, HttpCodes } from "@actions/http-client" +import { BearerCredentialHandler } from "@actions/http-client/lib/auth" +import { info } from "@actions/core" +import { ArtifactServiceClientJSON } from "src/generated" +import { getResultsServiceUrl, getRuntimeToken } from "./config" + +// The twirp http client must implement this interface +interface Rpc { + request( + service: string, + method: string, + contentType: "application/json" | "application/protobuf", + data: object | Uint8Array + ): Promise +} + +class ArtifactHttpClient implements Rpc { + private httpClient: HttpClient + private baseUrl: string + private maxAttempts: number = 5 + private baseRetryIntervalMilliseconds: number = 3000 + private retryMultiplier: number = 1.5 + + constructor(userAgent: string) { + const token = getRuntimeToken() + this.httpClient = new HttpClient( + userAgent, + [new BearerCredentialHandler(token)], + ) + this.baseUrl = getResultsServiceUrl() + } + + // This function satisfies the Rpc interface. It is compatible with the JSON + // JSON generated client. + async request( + service: string, + method: string, + contentType: "application/json" | "application/protobuf", + data: object | Uint8Array + ): Promise { + let url = `${this.baseUrl}/twirp/${service}/${method}` + let headers = { + "Content-Type": contentType, + } + + const response = await this.retryableRequest(this.httpClient.post(url, JSON.stringify(data), headers)) + const body = await response.readBody() + return JSON.parse(body) + } + + async retryableRequest( + operation: Promise + ): Promise { + let attempt = 0 + while (attempt < this.maxAttempts) { + let isRetryable = false + let errorMessage = "" + + try { + const response = await operation + const statusCode = response.message.statusCode + + if (this.isSuccessStatusCode(statusCode)) { + return response + } + + isRetryable = this.isRetryableHttpStatusCode(statusCode) + errorMessage = `Failed request: (${statusCode}) ${response.message.statusMessage}` + } catch (error) { + isRetryable = true + errorMessage = error.message + } + + if (!isRetryable) { + throw new Error(errorMessage) + } + + info( + `Attempt ${attempt + 1} of ${this.maxAttempts} failed with error: ${errorMessage}. Retrying request...` + ) + + await this.sleep(this.getExponentialRetryTimeMilliseconds(attempt)) + attempt++ + } + + throw new Error(`Failed to make request after ${this.maxAttempts} attempts`) + } + + isSuccessStatusCode(statusCode?: number): boolean { + if (!statusCode) return false + return statusCode >= 200 && statusCode < 300 + } + + isRetryableHttpStatusCode(statusCode?: number): boolean { + if (!statusCode) return false + + const retryableStatusCodes = [ + HttpCodes.BadGateway, + HttpCodes.GatewayTimeout, + HttpCodes.InternalServerError, + HttpCodes.ServiceUnavailable, + HttpCodes.TooManyRequests, + 413 // Payload Too Large + ] + + return retryableStatusCodes.includes(statusCode) + } + + async sleep(milliseconds: number): Promise { + return new Promise(resolve => setTimeout(resolve, milliseconds)) + } + + getExponentialRetryTimeMilliseconds(attempt: number): number { + if (attempt < 0) { + throw new Error("attempt should be a positive integer") + } + + if (attempt === 0) { + return this.baseRetryIntervalMilliseconds + } + + const minTime = this.baseRetryIntervalMilliseconds * this.retryMultiplier ** (attempt) + const maxTime = minTime * this.retryMultiplier + + // returns a random number between minTime and maxTime (exclusive) + return Math.trunc(Math.random() * (maxTime - minTime) + minTime) + } +} + +export function createArtifactTwirpClient(type: "upload" | "download"): ArtifactServiceClientJSON { + const client = new ArtifactHttpClient(`@actions/artifact-${type}`) + return new ArtifactServiceClientJSON(client) +} diff --git a/packages/artifact/src/internal/shared/config.ts b/packages/artifact/src/internal/shared/config.ts new file mode 100644 index 00000000..e0d41ba5 --- /dev/null +++ b/packages/artifact/src/internal/shared/config.ts @@ -0,0 +1,15 @@ +export function getRuntimeToken(): string { + const token = process.env['ACTIONS_RUNTIME_TOKEN'] + if (!token) { + throw new Error('Unable to get ACTIONS_RUNTIME_TOKEN env variable') + } + return token +} + +export function getResultsServiceUrl(): string { + const resultsUrl = process.env['ACTIONS_RESULTS_URL'] + if (!resultsUrl) { + throw new Error('Unable to get ACTIONS_RESULTS_URL env variable') + } + return resultsUrl +} From 4c6d88f93a6f017e3ec86d51aebc8250f72ff075 Mon Sep 17 00:00:00 2001 From: Bethany Date: Fri, 4 Aug 2023 13:00:58 -0700 Subject: [PATCH 02/15] Start writing tests --- .../__tests__/artifact-http-client.test.ts | 34 +++++++++++++++++++ .../internal/shared/artifact-twirp-client.ts | 2 +- tsconfig.json | 4 +-- 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 packages/artifact/__tests__/artifact-http-client.test.ts diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts new file mode 100644 index 00000000..3ab79116 --- /dev/null +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -0,0 +1,34 @@ +import * as config from "../src/internal/shared/config" +import { createArtifactTwirpClient } from "../src/internal/shared/artifact-twirp-client" +import * as core from "@actions/core" + +jest.mock("@actions/http-client") + +describe("artifact-http-client", () => { + beforeAll(() => { + // mock all output so that there is less noise when running tests + jest.spyOn(console, "log").mockImplementation(() => {}) + jest.spyOn(core, "debug").mockImplementation(() => {}) + jest.spyOn(core, "info").mockImplementation(() => {}) + jest.spyOn(core, "warning").mockImplementation(() => {}) + }) + + it("should successfully create a client", () => { + jest.spyOn(config, "getResultsServiceUrl").mockReturnValue("http://localhost:8080") + jest.spyOn(config, "getRuntimeToken").mockReturnValue("token") + const client = createArtifactTwirpClient("upload") + expect(client).toBeDefined() + }) + + it("should make a request", async () => { + const client = createArtifactTwirpClient("upload") + const artifact = client.CreateArtifact( + { + workflowRunBackendId: "1234", + workflowJobRunBackendId: "5678", + name: "artifact", + version: 4} + ) + expect(artifact).toBeDefined() + }) +}) diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index 02ac36c3..6e9345ea 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -1,7 +1,7 @@ import { HttpClient, HttpClientResponse, HttpCodes } from "@actions/http-client" import { BearerCredentialHandler } from "@actions/http-client/lib/auth" import { info } from "@actions/core" -import { ArtifactServiceClientJSON } from "src/generated" +import { ArtifactServiceClientJSON } from "../../generated" import { getResultsServiceUrl, getRuntimeToken } from "./config" // The twirp http client must implement this interface diff --git a/tsconfig.json b/tsconfig.json index fb610c6e..f706470a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "module": "commonjs", "strict": true, "declaration": true, - "target": "es6", + "target": "es2020", "sourceMap": true, "noImplicitAny": false, "baseUrl": "./", @@ -22,4 +22,4 @@ "**/*.test.ts", "**/__mocks__/*" ] -} \ No newline at end of file +} From efcab31d382deff0c414bd974084bd94c1d1ee82 Mon Sep 17 00:00:00 2001 From: Bethany Date: Mon, 7 Aug 2023 08:43:39 -0700 Subject: [PATCH 03/15] pass in http client to constructor --- .../__tests__/artifact-http-client.test.ts | 7 ++++--- .../internal/shared/artifact-twirp-client.ts | 18 +++++++++++------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 3ab79116..14c0a9cf 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -11,11 +11,11 @@ describe("artifact-http-client", () => { jest.spyOn(core, "debug").mockImplementation(() => {}) jest.spyOn(core, "info").mockImplementation(() => {}) jest.spyOn(core, "warning").mockImplementation(() => {}) + jest.spyOn(config, "getResultsServiceUrl").mockReturnValue("http://localhost:8080") + jest.spyOn(config, "getRuntimeToken").mockReturnValue("token") }) it("should successfully create a client", () => { - jest.spyOn(config, "getResultsServiceUrl").mockReturnValue("http://localhost:8080") - jest.spyOn(config, "getRuntimeToken").mockReturnValue("token") const client = createArtifactTwirpClient("upload") expect(client).toBeDefined() }) @@ -27,7 +27,8 @@ describe("artifact-http-client", () => { workflowRunBackendId: "1234", workflowJobRunBackendId: "5678", name: "artifact", - version: 4} + version: 4 + } ) expect(artifact).toBeDefined() }) diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index 6e9345ea..7241b0c5 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -21,13 +21,17 @@ class ArtifactHttpClient implements Rpc { private baseRetryIntervalMilliseconds: number = 3000 private retryMultiplier: number = 1.5 - constructor(userAgent: string) { + constructor(userAgent: string, httpClient?: HttpClient) { const token = getRuntimeToken() - this.httpClient = new HttpClient( - userAgent, - [new BearerCredentialHandler(token)], - ) this.baseUrl = getResultsServiceUrl() + if (httpClient) { + this.httpClient = httpClient + } else { + this.httpClient = new HttpClient( + userAgent, + [new BearerCredentialHandler(token)], + ) + } } // This function satisfies the Rpc interface. It is compatible with the JSON @@ -127,7 +131,7 @@ class ArtifactHttpClient implements Rpc { } } -export function createArtifactTwirpClient(type: "upload" | "download"): ArtifactServiceClientJSON { - const client = new ArtifactHttpClient(`@actions/artifact-${type}`) +export function createArtifactTwirpClient(type: "upload" | "download", httpClient?: HttpClient): ArtifactServiceClientJSON { + const client = new ArtifactHttpClient(`@actions/artifact-${type}`, httpClient) return new ArtifactServiceClientJSON(client) } From c608703ecf826501a70ea57e72fcf95254e6d8b2 Mon Sep 17 00:00:00 2001 From: Bethany Date: Mon, 7 Aug 2023 08:48:15 -0700 Subject: [PATCH 04/15] revert root tsconfig.json --- tsconfig.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tsconfig.json b/tsconfig.json index f706470a..fb610c6e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "module": "commonjs", "strict": true, "declaration": true, - "target": "es2020", + "target": "es6", "sourceMap": true, "noImplicitAny": false, "baseUrl": "./", @@ -22,4 +22,4 @@ "**/*.test.ts", "**/__mocks__/*" ] -} +} \ No newline at end of file From 6552cb9722baba43c2e2a263aaee3c146dadb621 Mon Sep 17 00:00:00 2001 From: Bethany Date: Mon, 7 Aug 2023 14:24:58 -0700 Subject: [PATCH 05/15] wip --- package-lock.json | 132 ++++++++++++++++ package.json | 3 +- .../__tests__/artifact-http-client.test.ts | 142 +++++++++++++++++- tsconfig.json | 4 +- 4 files changed, 275 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 79c3ae90..e8367a9a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,6 +18,7 @@ "flow-bin": "^0.115.0", "jest": "^27.2.5", "lerna": "^7.1.4", + "node-mocks-http": "^1.12.2", "nx": "16.6.0", "prettier": "^3.0.0", "ts-jest": "^27.0.5", @@ -2564,6 +2565,19 @@ "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", "dev": true }, + "node_modules/accepts": { + "version": "1.3.8", + "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", + "integrity": "sha512-PYAthTa2m2VKxuvSD3DPC/Gy+U+sOA1LAuT8mkmRuvw+NACSaeXEQ+NHcVF7rONl6qcaxV3Uuemwawk+7+SJLw==", + "dev": true, + "dependencies": { + "mime-types": "~2.1.34", + "negotiator": "0.6.3" + }, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", @@ -3752,6 +3766,18 @@ "integrity": "sha512-ty/fTekppD2fIwRvnZAVdeOiGd1c7YXEixbgJTNzqcxJWKQnjJ/V1bNEEE6hygpM3WjwHFUVK6HTjWSzV4a8sQ==", "dev": true }, + "node_modules/content-disposition": { + "version": "0.5.4", + "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz", + "integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==", + "dev": true, + "dependencies": { + "safe-buffer": "5.2.1" + }, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/conventional-changelog-angular": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/conventional-changelog-angular/-/conventional-changelog-angular-6.0.0.tgz", @@ -5600,6 +5626,15 @@ "node": ">= 6" } }, + "node_modules/fresh": { + "version": "0.5.2", + "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.5.2.tgz", + "integrity": "sha512-zJ2mQYM18rEFOudeV4GShTGIQ7RbzA7ozbU9I/XBpm7kqgMywgmylMwXHxZJmkVoYkna9d2pVXVXPdYTP9ej8Q==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -8697,6 +8732,15 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/media-typer": { + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", + "integrity": "sha512-dq+qelQ9akHpcOl/gUVRTxVIOkAJ1wR3QAvb4RsVjS8oVoFjDGTc679wJYmUmknUF5HwMLOgb5O+a3KxfWapPQ==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/meow": { "version": "8.1.2", "resolved": "https://registry.npmjs.org/meow/-/meow-8.1.2.tgz", @@ -8863,6 +8907,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/merge-descriptors": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", + "integrity": "sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w==", + "dev": true + }, "node_modules/merge-stream": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/merge-stream/-/merge-stream-2.0.0.tgz", @@ -8878,6 +8928,15 @@ "node": ">= 8" } }, + "node_modules/methods": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz", + "integrity": "sha512-iclAHeNqNm68zFtnZ0e+1L2yUIdvzNoauKU4WBA3VvH/vPFieF7qfRlwUZU+DA9P9bPXIS90ulxoUoCH23sV2w==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/micromatch": { "version": "4.0.5", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", @@ -8891,6 +8950,18 @@ "node": ">=8.6" } }, + "node_modules/mime": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/mime/-/mime-1.6.0.tgz", + "integrity": "sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg==", + "dev": true, + "bin": { + "mime": "cli.js" + }, + "engines": { + "node": ">=4" + } + }, "node_modules/mime-db": { "version": "1.52.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", @@ -9354,6 +9425,36 @@ "integrity": "sha512-QNABxbrPa3qEIfrE6GOJ7BYIuignnJw7iQ2YPbc3Nla1HzRJjXzZOiikfF8m7eAMfichLt3M4VgLOetqgDmgGQ==", "dev": true }, + "node_modules/node-mocks-http": { + "version": "1.12.2", + "resolved": "https://registry.npmjs.org/node-mocks-http/-/node-mocks-http-1.12.2.tgz", + "integrity": "sha512-xhWwC0dh35R9rf0j3bRZXuISXdHxxtMx0ywZQBwjrg3yl7KpRETzogfeCamUIjltpn0Fxvs/ZhGJul1vPLrdJQ==", + "dev": true, + "dependencies": { + "accepts": "^1.3.7", + "content-disposition": "^0.5.3", + "depd": "^1.1.0", + "fresh": "^0.5.2", + "merge-descriptors": "^1.0.1", + "methods": "^1.1.2", + "mime": "^1.3.4", + "parseurl": "^1.3.3", + "range-parser": "^1.2.0", + "type-is": "^1.6.18" + }, + "engines": { + "node": ">=0.6" + } + }, + "node_modules/node-mocks-http/node_modules/depd": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/depd/-/depd-1.1.2.tgz", + "integrity": "sha512-7emPTl6Dpo6JRXOXjLRxck+FlLRX5847cLKEn00PLAgc3g2hTZZgr+e4c2v6QpSmLeFP3n5yUo7ft6avBK/5jQ==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/node-releases": { "version": "2.0.13", "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.13.tgz", @@ -10500,6 +10601,15 @@ "integrity": "sha512-Ofn/CTFzRGTTxwpNEs9PP93gXShHcTq255nzRYSKe8AkVpZY7e1fpmTfOyoIvjP5HG7Z2ZM7VS9PPhQGW2pOpw==", "dev": true }, + "node_modules/parseurl": { + "version": "1.3.3", + "resolved": "https://registry.npmjs.org/parseurl/-/parseurl-1.3.3.tgz", + "integrity": "sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ==", + "dev": true, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/path-exists": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", @@ -10853,6 +10963,15 @@ "node": ">=8" } }, + "node_modules/range-parser": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz", + "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==", + "dev": true, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/react-is": { "version": "17.0.2", "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", @@ -12527,6 +12646,19 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/type-is": { + "version": "1.6.18", + "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.18.tgz", + "integrity": "sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==", + "dev": true, + "dependencies": { + "media-typer": "0.3.0", + "mime-types": "~2.1.24" + }, + "engines": { + "node": ">= 0.6" + } + }, "node_modules/typed-array-buffer": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/typed-array-buffer/-/typed-array-buffer-1.0.0.tgz", diff --git a/package.json b/package.json index d64d2841..1b3e02e8 100644 --- a/package.json +++ b/package.json @@ -28,9 +28,10 @@ "flow-bin": "^0.115.0", "jest": "^27.2.5", "lerna": "^7.1.4", + "node-mocks-http": "^1.12.2", "nx": "16.6.0", "prettier": "^3.0.0", "ts-jest": "^27.0.5", "typescript": "^3.9.9" } -} \ No newline at end of file +} diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 14c0a9cf..44469ee6 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -1,8 +1,26 @@ +import * as http from "http" +import * as net from "net" +import { HttpClient } from "@actions/http-client" import * as config from "../src/internal/shared/config" import { createArtifactTwirpClient } from "../src/internal/shared/artifact-twirp-client" import * as core from "@actions/core" -jest.mock("@actions/http-client") + +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)} + } +}) +jest.mock("@actions/http-client", () => { + return jest.fn().mockImplementation(() => { + return { + post: mockPost + } + }) +}) describe("artifact-http-client", () => { beforeAll(() => { @@ -15,14 +33,33 @@ describe("artifact-http-client", () => { jest.spyOn(config, "getRuntimeToken").mockReturnValue("token") }) + beforeEach(() => { + mockPost.mockClear(); + (HttpClient as unknown as jest.Mock).mockClear() + }) + it("should successfully create a client", () => { const client = createArtifactTwirpClient("upload") expect(client).toBeDefined() }) it("should make a request", async () => { - const client = createArtifactTwirpClient("upload") - const artifact = client.CreateArtifact( + /* + const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + return { + post: () => { + const msg = new http.IncomingMessage(new net.Socket()) + msg.statusCode = 200 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/lol/upload"}`)} + } + } + } + }) + */ + const client = createArtifactTwirpClient("upload", new HttpClient()) + const artifact = await client.CreateArtifact( { workflowRunBackendId: "1234", workflowJobRunBackendId: "5678", @@ -30,6 +67,105 @@ describe("artifact-http-client", () => { version: 4 } ) + + expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() + expect(artifact.ok).toBe(true) + expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") + }) + + it("should retry if the request fails", async () => { + /* + const mockPost = jest.fn(() => { + const msg = new http.IncomingMessage(new net.Socket()) + if (mockPost.mock.calls.length > 1) { + msg.statusCode = 200 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} + } + } + msg.statusCode = 500 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": false}`)} + } + }) + */ + 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 msg = new http.IncomingMessage(new net.Socket()) + msg.statusCode = 500 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": false}`)} + } + }) + + mockPost.mockImplementation(() => { + const msg = new http.IncomingMessage(new net.Socket()) + msg.statusCode = 200 + return { + message: msg, + readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/lol/upload"}`)} + } + }) + */ + + jest.mock("@actions/http-client", () => { + return jest.fn().mockImplementation(() => { + return { + post: mockPost + } + }) + }) + /* + jest.mock("@actions/http-client", () => { + return { + HttpClient: jest.fn().mockImplementation(() => { + return { + post: mockPost + } + }) + } + }) + jest.mock("@actions/http-client", () => { + return jest.fn().mockImplementation(() => { + return { + post: mockPost + } + }) + }) + */ + + const client = createArtifactTwirpClient("upload", new HttpClient()) + const artifact = await client.CreateArtifact( + { + workflowRunBackendId: "1234", + workflowJobRunBackendId: "5678", + name: "artifact", + version: 4 + } + ) + + expect(artifact).toBeDefined() + expect(artifact.ok).toBe(true) + expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") + expect(mockPost).toHaveBeenCalledTimes(2) }) }) + diff --git a/tsconfig.json b/tsconfig.json index fb610c6e..f706470a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "module": "commonjs", "strict": true, "declaration": true, - "target": "es6", + "target": "es2020", "sourceMap": true, "noImplicitAny": false, "baseUrl": "./", @@ -22,4 +22,4 @@ "**/*.test.ts", "**/__mocks__/*" ] -} \ No newline at end of file +} From af1621025dca6ff458b308b57d70dbd0c0355f9b Mon Sep 17 00:00:00 2001 From: Bethany Date: Mon, 7 Aug 2023 16:26:07 -0700 Subject: [PATCH 06/15] 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) } From e85cd96d85c1b2d45b827b43ef44538a6a509b33 Mon Sep 17 00:00:00 2001 From: Bethany Date: Tue, 8 Aug 2023 12:49:05 -0700 Subject: [PATCH 07/15] tests and fix bug for retry --- .../__tests__/artifact-http-client.test.ts | 190 +++++++++--------- .../internal/shared/artifact-twirp-client.ts | 52 +++-- 2 files changed, 136 insertions(+), 106 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 68495ce2..0477b731 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -5,17 +5,7 @@ import * as config from "../src/internal/shared/config" import { createArtifactTwirpClient } from "../src/internal/shared/artifact-twirp-client" import * as core from "@actions/core" - -const mockPost = jest.fn() -jest.mock("@actions/http-client", () => { - return { - HttpClient: jest.fn().mockImplementation(() => { - return { - post: mockPost - } - }) - } -}) +jest.mock("@actions/http-client") describe("artifact-http-client", () => { beforeAll(() => { @@ -29,8 +19,7 @@ describe("artifact-http-client", () => { }) beforeEach(() => { - mockPost.mockClear(); - + jest.clearAllMocks() }) it("should successfully create a client", () => { @@ -39,21 +28,7 @@ describe("artifact-http-client", () => { }) it("should make a request", async () => { - /* - const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { - return { - post: () => { - const msg = new http.IncomingMessage(new net.Socket()) - msg.statusCode = 200 - return { - message: msg, - readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/lol/upload"}`)} - } - } - } - }) - */ - mockPost.mockImplementationOnce(() => { + const mockPost = jest.fn(() => { const msg = new http.IncomingMessage(new net.Socket()) msg.statusCode = 200 return { @@ -61,6 +36,11 @@ describe("artifact-http-client", () => { readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} } }) + const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + return { + post: mockPost + } + }) const client = createArtifactTwirpClient("upload") const artifact = await client.CreateArtifact( @@ -72,6 +52,7 @@ describe("artifact-http-client", () => { } ) + expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(mockPost).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() expect(artifact.ok).toBe(true) @@ -79,32 +60,8 @@ describe("artifact-http-client", () => { }) it("should retry if the request fails", async () => { - /* - const mockPost = jest.fn(() => { - const msg = new http.IncomingMessage(new net.Socket()) - if (mockPost.mock.calls.length > 1) { - msg.statusCode = 200 - return { - message: msg, - readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} - } - } - msg.statusCode = 500 - return { - message: msg, - readBody: () => {return Promise.resolve(`{"ok": false}`)} - } - }) - */ - - mockPost.mockImplementationOnce(() => { - const msgFailed = new http.IncomingMessage(new net.Socket()) - msgFailed.statusCode = 500 - return { - message: msgFailed, - readBody: () => {return Promise.resolve(`{"ok": false}`)} - } - }).mockImplementationOnce(() => { + const mockPost = jest + .fn(() => { const msgSucceeded = new http.IncomingMessage(new net.Socket()) msgSucceeded.statusCode = 200 return { @@ -112,51 +69,27 @@ describe("artifact-http-client", () => { readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}`)} } }) - - /* - mockPost.mockImplementationOnce(() => { - const msg = new http.IncomingMessage(new net.Socket()) - msg.statusCode = 500 + .mockImplementationOnce(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 500 + msgFailed.statusMessage = "Internal Server Error" return { - message: msg, + message: msgFailed, readBody: () => {return Promise.resolve(`{"ok": false}`)} - } - }) - - mockPost.mockImplementation(() => { - const msg = new http.IncomingMessage(new net.Socket()) - msg.statusCode = 200 - return { - message: msg, - readBody: () => {return Promise.resolve(`{"ok": true, "signedUploadUrl": "http://localhost:8080/lol/upload"}`)} } }) - jest.mock("@actions/http-client", () => { - return jest.fn().mockImplementation(() => { - return { - post: mockPost - } - }) - }) - jest.mock("@actions/http-client", () => { + const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { return { - HttpClient: jest.fn().mockImplementation(() => { - return { - post: mockPost - } - }) + post: mockPost } }) - jest.mock("@actions/http-client", () => { - return jest.fn().mockImplementation(() => { - return { - post: mockPost - } - }) - }) - */ - const client = createArtifactTwirpClient("upload") + const client = createArtifactTwirpClient( + "upload", + 5, // retry 5 times + 1, // wait 1 ms + 1.5 // backoff factor + ) const artifact = await client.CreateArtifact( { workflowRunBackendId: "1234", @@ -166,10 +99,85 @@ describe("artifact-http-client", () => { } ) + expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() expect(artifact.ok).toBe(true) expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") expect(mockPost).toHaveBeenCalledTimes(2) }) + + it("should fail if the request fails 5 times", async () => { + const mockPost = jest + .fn(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 500 + msgFailed.statusMessage = "Internal Server Error" + return { + message: msgFailed, + readBody: () => {return Promise.resolve(`{"ok": false}`)} + } + }) + + const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + return { + post: mockPost + } + }) + const client = createArtifactTwirpClient( + "upload", + 5, // retry 5 times + 1, // wait 1 ms + 1.5 // backoff factor + ) + await expect(async () => { + await client.CreateArtifact( + { + workflowRunBackendId: "1234", + workflowJobRunBackendId: "5678", + name: "artifact", + version: 4 + } + ) + }).rejects.toThrowError("Failed to make request after 5 attempts: Failed request: (500) Internal Server Error") + expect(mockHttpClient).toHaveBeenCalledTimes(1) + expect(mockPost).toHaveBeenCalledTimes(5) + }) + + it("should fail immediately if there is a non-retryable error", async () => { + const mockPost = jest + .fn(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 401 + msgFailed.statusMessage = "Unauthorized" + return { + message: msgFailed, + readBody: () => {return Promise.resolve(`{"ok": false}`)} + } + }) + + const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + return { + post: mockPost + } + }) + const client = createArtifactTwirpClient( + "upload", + 5, // retry 5 times + 1, // wait 1 ms + 1.5 // backoff factor + ) + await expect(async () => { + await client.CreateArtifact( + { + workflowRunBackendId: "1234", + workflowJobRunBackendId: "5678", + name: "artifact", + version: 4 + } + ) + }).rejects.toThrowError("Received non-retryable error: Failed request: (401) Unauthorized") + 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 a8e989b0..97b1b5f6 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -21,9 +21,19 @@ class ArtifactHttpClient implements Rpc { private baseRetryIntervalMilliseconds: number = 3000 private retryMultiplier: number = 1.5 - constructor(userAgent: string) { + constructor(userAgent: string, maxAttempts?: number, baseRetryIntervalMilliseconds?: number, retryMultiplier?: number) { const token = getRuntimeToken() this.baseUrl = getResultsServiceUrl() + if (maxAttempts) { + this.maxAttempts = maxAttempts + } + if (baseRetryIntervalMilliseconds) { + this.baseRetryIntervalMilliseconds = baseRetryIntervalMilliseconds + } + if (retryMultiplier) { + this.retryMultiplier = retryMultiplier + } + this.httpClient = new HttpClient( userAgent, [new BearerCredentialHandler(token)], @@ -42,22 +52,27 @@ class ArtifactHttpClient implements Rpc { let headers = { "Content-Type": contentType, } + info(`Making request to ${url} with data: ${JSON.stringify(data)}`) - const response = await this.retryableRequest(this.httpClient.post(url, JSON.stringify(data), headers)) - const body = await response.readBody() - return JSON.parse(body) + try { + const response = await this.retryableRequest(() => this.httpClient.post(url, JSON.stringify(data), headers)) + const body = await response.readBody() + return JSON.parse(body) + } catch (error) { + throw new Error(error.message) + } } async retryableRequest( - operation: Promise + operation: () => Promise, ): Promise { let attempt = 0 + let errorMessage = "" while (attempt < this.maxAttempts) { let isRetryable = false - let errorMessage = "" try { - const response = await operation + const response = await operation() const statusCode = response.message.statusCode if (this.isSuccessStatusCode(statusCode)) { @@ -72,25 +87,22 @@ class ArtifactHttpClient implements Rpc { } if (!isRetryable) { - throw new Error(errorMessage) + throw new Error(`Received non-retryable error: ${errorMessage}`) } if (attempt + 1 === this.maxAttempts) { - info(`Final attempt failed with error: ${errorMessage}`) - break + throw new Error(`Failed to make request after ${this.maxAttempts} attempts: ${errorMessage}`) } const retryTimeMilliseconds = this.getExponentialRetryTimeMilliseconds(attempt) - info( `Attempt ${attempt + 1} of ${this.maxAttempts} failed with error: ${errorMessage}. Retrying request in ${retryTimeMilliseconds} ms...` ) - await this.sleep(retryTimeMilliseconds) attempt++ } - throw new Error(`Failed to make request after ${this.maxAttempts} attempts`) + throw new Error(`Request failed`) } isSuccessStatusCode(statusCode?: number): boolean { @@ -134,7 +146,17 @@ class ArtifactHttpClient implements Rpc { } } -export function createArtifactTwirpClient(type: "upload" | "download"): ArtifactServiceClientJSON { - const client = new ArtifactHttpClient(`@actions/artifact-${type}`) +export function createArtifactTwirpClient( + type: "upload" | "download", + maxAttempts?: number, + baseRetryIntervalMilliseconds?: number, + retryMultiplier?: number +): ArtifactServiceClientJSON { + const client = new ArtifactHttpClient( + `@actions/artifact-${type}`, + maxAttempts, + baseRetryIntervalMilliseconds, + retryMultiplier + ) return new ArtifactServiceClientJSON(client) } From 1718e0d97cb0b0cdf57cbb106e9a91e969a1bf68 Mon Sep 17 00:00:00 2001 From: Bethany Date: Tue, 8 Aug 2023 12:49:45 -0700 Subject: [PATCH 08/15] revert target to es6 --- tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index f706470a..14c7f781 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,7 +4,7 @@ "module": "commonjs", "strict": true, "declaration": true, - "target": "es2020", + "target": "es6", "sourceMap": true, "noImplicitAny": false, "baseUrl": "./", From ad9b955fe98f6d7086371878c3f75f07b71e9776 Mon Sep 17 00:00:00 2001 From: Bethany Date: Tue, 8 Aug 2023 12:51:54 -0700 Subject: [PATCH 09/15] remove unused package --- package-lock.json | 132 ---------------------------------------------- package.json | 1 - 2 files changed, 133 deletions(-) diff --git a/package-lock.json b/package-lock.json index e8367a9a..79c3ae90 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,7 +18,6 @@ "flow-bin": "^0.115.0", "jest": "^27.2.5", "lerna": "^7.1.4", - "node-mocks-http": "^1.12.2", "nx": "16.6.0", "prettier": "^3.0.0", "ts-jest": "^27.0.5", @@ -2565,19 +2564,6 @@ "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", "dev": true }, - "node_modules/accepts": { - "version": "1.3.8", - "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", - "integrity": "sha512-PYAthTa2m2VKxuvSD3DPC/Gy+U+sOA1LAuT8mkmRuvw+NACSaeXEQ+NHcVF7rONl6qcaxV3Uuemwawk+7+SJLw==", - "dev": true, - "dependencies": { - "mime-types": "~2.1.34", - "negotiator": "0.6.3" - }, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", @@ -3766,18 +3752,6 @@ "integrity": "sha512-ty/fTekppD2fIwRvnZAVdeOiGd1c7YXEixbgJTNzqcxJWKQnjJ/V1bNEEE6hygpM3WjwHFUVK6HTjWSzV4a8sQ==", "dev": true }, - "node_modules/content-disposition": { - "version": "0.5.4", - "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz", - "integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==", - "dev": true, - "dependencies": { - "safe-buffer": "5.2.1" - }, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/conventional-changelog-angular": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/conventional-changelog-angular/-/conventional-changelog-angular-6.0.0.tgz", @@ -5626,15 +5600,6 @@ "node": ">= 6" } }, - "node_modules/fresh": { - "version": "0.5.2", - "resolved": "https://registry.npmjs.org/fresh/-/fresh-0.5.2.tgz", - "integrity": "sha512-zJ2mQYM18rEFOudeV4GShTGIQ7RbzA7ozbU9I/XBpm7kqgMywgmylMwXHxZJmkVoYkna9d2pVXVXPdYTP9ej8Q==", - "dev": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -8732,15 +8697,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/media-typer": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", - "integrity": "sha512-dq+qelQ9akHpcOl/gUVRTxVIOkAJ1wR3QAvb4RsVjS8oVoFjDGTc679wJYmUmknUF5HwMLOgb5O+a3KxfWapPQ==", - "dev": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/meow": { "version": "8.1.2", "resolved": "https://registry.npmjs.org/meow/-/meow-8.1.2.tgz", @@ -8907,12 +8863,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/merge-descriptors": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-1.0.1.tgz", - "integrity": "sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w==", - "dev": true - }, "node_modules/merge-stream": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/merge-stream/-/merge-stream-2.0.0.tgz", @@ -8928,15 +8878,6 @@ "node": ">= 8" } }, - "node_modules/methods": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz", - "integrity": "sha512-iclAHeNqNm68zFtnZ0e+1L2yUIdvzNoauKU4WBA3VvH/vPFieF7qfRlwUZU+DA9P9bPXIS90ulxoUoCH23sV2w==", - "dev": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/micromatch": { "version": "4.0.5", "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", @@ -8950,18 +8891,6 @@ "node": ">=8.6" } }, - "node_modules/mime": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/mime/-/mime-1.6.0.tgz", - "integrity": "sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg==", - "dev": true, - "bin": { - "mime": "cli.js" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/mime-db": { "version": "1.52.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", @@ -9425,36 +9354,6 @@ "integrity": "sha512-QNABxbrPa3qEIfrE6GOJ7BYIuignnJw7iQ2YPbc3Nla1HzRJjXzZOiikfF8m7eAMfichLt3M4VgLOetqgDmgGQ==", "dev": true }, - "node_modules/node-mocks-http": { - "version": "1.12.2", - "resolved": "https://registry.npmjs.org/node-mocks-http/-/node-mocks-http-1.12.2.tgz", - "integrity": "sha512-xhWwC0dh35R9rf0j3bRZXuISXdHxxtMx0ywZQBwjrg3yl7KpRETzogfeCamUIjltpn0Fxvs/ZhGJul1vPLrdJQ==", - "dev": true, - "dependencies": { - "accepts": "^1.3.7", - "content-disposition": "^0.5.3", - "depd": "^1.1.0", - "fresh": "^0.5.2", - "merge-descriptors": "^1.0.1", - "methods": "^1.1.2", - "mime": "^1.3.4", - "parseurl": "^1.3.3", - "range-parser": "^1.2.0", - "type-is": "^1.6.18" - }, - "engines": { - "node": ">=0.6" - } - }, - "node_modules/node-mocks-http/node_modules/depd": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/depd/-/depd-1.1.2.tgz", - "integrity": "sha512-7emPTl6Dpo6JRXOXjLRxck+FlLRX5847cLKEn00PLAgc3g2hTZZgr+e4c2v6QpSmLeFP3n5yUo7ft6avBK/5jQ==", - "dev": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/node-releases": { "version": "2.0.13", "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.13.tgz", @@ -10601,15 +10500,6 @@ "integrity": "sha512-Ofn/CTFzRGTTxwpNEs9PP93gXShHcTq255nzRYSKe8AkVpZY7e1fpmTfOyoIvjP5HG7Z2ZM7VS9PPhQGW2pOpw==", "dev": true }, - "node_modules/parseurl": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/parseurl/-/parseurl-1.3.3.tgz", - "integrity": "sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ==", - "dev": true, - "engines": { - "node": ">= 0.8" - } - }, "node_modules/path-exists": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", @@ -10963,15 +10853,6 @@ "node": ">=8" } }, - "node_modules/range-parser": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz", - "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==", - "dev": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/react-is": { "version": "17.0.2", "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", @@ -12646,19 +12527,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/type-is": { - "version": "1.6.18", - "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.18.tgz", - "integrity": "sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==", - "dev": true, - "dependencies": { - "media-typer": "0.3.0", - "mime-types": "~2.1.24" - }, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/typed-array-buffer": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/typed-array-buffer/-/typed-array-buffer-1.0.0.tgz", diff --git a/package.json b/package.json index 1b3e02e8..20221837 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,6 @@ "flow-bin": "^0.115.0", "jest": "^27.2.5", "lerna": "^7.1.4", - "node-mocks-http": "^1.12.2", "nx": "16.6.0", "prettier": "^3.0.0", "ts-jest": "^27.0.5", From c0684c5add60283cf5b0f5a6369b283fe000a9af Mon Sep 17 00:00:00 2001 From: Bethany Date: Tue, 8 Aug 2023 13:19:43 -0700 Subject: [PATCH 10/15] prettier --- .../__tests__/artifact-http-client.test.ts | 259 ++++++++++-------- .../internal/shared/artifact-twirp-client.ts | 62 +++-- 2 files changed, 175 insertions(+), 146 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 0477b731..91327114 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -1,183 +1,200 @@ -import * as http from "http" -import * as net from "net" -import { HttpClient } from "@actions/http-client" -import * as config from "../src/internal/shared/config" -import { createArtifactTwirpClient } from "../src/internal/shared/artifact-twirp-client" -import * as core from "@actions/core" +import * as http from 'http' +import * as net from 'net' +import {HttpClient} from '@actions/http-client' +import * as config from '../src/internal/shared/config' +import {createArtifactTwirpClient} from '../src/internal/shared/artifact-twirp-client' +import * as core from '@actions/core' -jest.mock("@actions/http-client") +jest.mock('@actions/http-client') -describe("artifact-http-client", () => { +describe('artifact-http-client', () => { beforeAll(() => { // mock all output so that there is less noise when running tests - jest.spyOn(console, "log").mockImplementation(() => {}) - jest.spyOn(core, "debug").mockImplementation(() => {}) - jest.spyOn(core, "info").mockImplementation(() => {}) - jest.spyOn(core, "warning").mockImplementation(() => {}) - jest.spyOn(config, "getResultsServiceUrl").mockReturnValue("http://localhost:8080") - jest.spyOn(config, "getRuntimeToken").mockReturnValue("token") + jest.spyOn(console, 'log').mockImplementation(() => {}) + jest.spyOn(core, 'debug').mockImplementation(() => {}) + jest.spyOn(core, 'info').mockImplementation(() => {}) + jest.spyOn(core, 'warning').mockImplementation(() => {}) + jest + .spyOn(config, 'getResultsServiceUrl') + .mockReturnValue('http://localhost:8080') + jest.spyOn(config, 'getRuntimeToken').mockReturnValue('token') }) beforeEach(() => { jest.clearAllMocks() }) - it("should successfully create a client", () => { - const client = createArtifactTwirpClient("upload") + it('should successfully create a client', () => { + const client = createArtifactTwirpClient('upload') expect(client).toBeDefined() }) - it("should make a request", async () => { + it('should make a request', async () => { const mockPost = jest.fn(() => { 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"}`)} + readBody: () => { + return Promise.resolve( + `{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}` + ) + } } }) - const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + const mockHttpClient = ( + HttpClient as unknown as jest.Mock + ).mockImplementation(() => { return { post: mockPost } }) - const client = createArtifactTwirpClient("upload") - const artifact = await client.CreateArtifact( - { - workflowRunBackendId: "1234", - workflowJobRunBackendId: "5678", - name: "artifact", - version: 4 - } - ) + const client = createArtifactTwirpClient('upload') + const artifact = await client.CreateArtifact({ + workflowRunBackendId: '1234', + workflowJobRunBackendId: '5678', + name: 'artifact', + version: 4 + }) - expect(mockHttpClient).toHaveBeenCalledTimes(1) + expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(mockPost).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() expect(artifact.ok).toBe(true) - expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") + expect(artifact.signedUploadUrl).toBe('http://localhost:8080/upload') }) - it("should retry if the request fails", async () => { + it('should retry if the request fails', async () => { const mockPost = jest - .fn(() => { - 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"}`)} - } - }) - .mockImplementationOnce(() => { - const msgFailed = new http.IncomingMessage(new net.Socket()) - msgFailed.statusCode = 500 - msgFailed.statusMessage = "Internal Server Error" - return { - message: msgFailed, - readBody: () => {return Promise.resolve(`{"ok": false}`)} - } - }) - const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { + .fn(() => { + 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"}` + ) + } + } + }) + .mockImplementationOnce(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 500 + msgFailed.statusMessage = 'Internal Server Error' + return { + message: msgFailed, + readBody: () => { + return Promise.resolve(`{"ok": false}`) + } + } + }) + const mockHttpClient = ( + HttpClient as unknown as jest.Mock + ).mockImplementation(() => { return { post: mockPost } }) const client = createArtifactTwirpClient( - "upload", + 'upload', 5, // retry 5 times 1, // wait 1 ms 1.5 // backoff factor ) - const artifact = await client.CreateArtifact( - { - workflowRunBackendId: "1234", - workflowJobRunBackendId: "5678", - name: "artifact", - version: 4 - } - ) + const artifact = await client.CreateArtifact({ + workflowRunBackendId: '1234', + workflowJobRunBackendId: '5678', + name: 'artifact', + version: 4 + }) expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(artifact).toBeDefined() expect(artifact.ok).toBe(true) - expect(artifact.signedUploadUrl).toBe("http://localhost:8080/upload") + expect(artifact.signedUploadUrl).toBe('http://localhost:8080/upload') expect(mockPost).toHaveBeenCalledTimes(2) }) - it("should fail if the request fails 5 times", async () => { - const mockPost = jest - .fn(() => { - const msgFailed = new http.IncomingMessage(new net.Socket()) - msgFailed.statusCode = 500 - msgFailed.statusMessage = "Internal Server Error" - return { - message: msgFailed, - readBody: () => {return Promise.resolve(`{"ok": false}`)} + it('should fail if the request fails 5 times', async () => { + const mockPost = jest.fn(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 500 + msgFailed.statusMessage = 'Internal Server Error' + return { + message: msgFailed, + readBody: () => { + return Promise.resolve(`{"ok": false}`) } - }) + } + }) - const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { - return { - post: mockPost - } - }) - const client = createArtifactTwirpClient( - "upload", - 5, // retry 5 times - 1, // wait 1 ms - 1.5 // backoff factor - ) + const mockHttpClient = ( + HttpClient as unknown as jest.Mock + ).mockImplementation(() => { + return { + post: mockPost + } + }) + const client = createArtifactTwirpClient( + 'upload', + 5, // retry 5 times + 1, // wait 1 ms + 1.5 // backoff factor + ) await expect(async () => { - await client.CreateArtifact( - { - workflowRunBackendId: "1234", - workflowJobRunBackendId: "5678", - name: "artifact", - version: 4 - } - ) - }).rejects.toThrowError("Failed to make request after 5 attempts: Failed request: (500) Internal Server Error") + await client.CreateArtifact({ + workflowRunBackendId: '1234', + workflowJobRunBackendId: '5678', + name: 'artifact', + version: 4 + }) + }).rejects.toThrowError( + 'Failed to make request after 5 attempts: Failed request: (500) Internal Server Error' + ) expect(mockHttpClient).toHaveBeenCalledTimes(1) expect(mockPost).toHaveBeenCalledTimes(5) }) - - it("should fail immediately if there is a non-retryable error", async () => { - const mockPost = jest - .fn(() => { - const msgFailed = new http.IncomingMessage(new net.Socket()) - msgFailed.statusCode = 401 - msgFailed.statusMessage = "Unauthorized" - return { - message: msgFailed, - readBody: () => {return Promise.resolve(`{"ok": false}`)} - } - }) - const mockHttpClient = (HttpClient as unknown as jest.Mock).mockImplementation(() => { - return { - post: mockPost + it('should fail immediately if there is a non-retryable error', async () => { + const mockPost = jest.fn(() => { + const msgFailed = new http.IncomingMessage(new net.Socket()) + msgFailed.statusCode = 401 + msgFailed.statusMessage = 'Unauthorized' + return { + message: msgFailed, + readBody: () => { + return Promise.resolve(`{"ok": false}`) } - }) - const client = createArtifactTwirpClient( - "upload", - 5, // retry 5 times - 1, // wait 1 ms - 1.5 // backoff factor - ) + } + }) + + const mockHttpClient = ( + HttpClient as unknown as jest.Mock + ).mockImplementation(() => { + return { + post: mockPost + } + }) + const client = createArtifactTwirpClient( + 'upload', + 5, // retry 5 times + 1, // wait 1 ms + 1.5 // backoff factor + ) await expect(async () => { - await client.CreateArtifact( - { - workflowRunBackendId: "1234", - workflowJobRunBackendId: "5678", - name: "artifact", - version: 4 - } - ) - }).rejects.toThrowError("Received non-retryable error: Failed request: (401) Unauthorized") + await client.CreateArtifact({ + workflowRunBackendId: '1234', + workflowJobRunBackendId: '5678', + name: 'artifact', + version: 4 + }) + }).rejects.toThrowError( + 'Received non-retryable error: Failed request: (401) Unauthorized' + ) 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 97b1b5f6..e582db45 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -1,15 +1,15 @@ -import { HttpClient, HttpClientResponse, HttpCodes } from "@actions/http-client" -import { BearerCredentialHandler } from "@actions/http-client/lib/auth" -import { info } from "@actions/core" -import { ArtifactServiceClientJSON } from "../../generated" -import { getResultsServiceUrl, getRuntimeToken } from "./config" +import {HttpClient, HttpClientResponse, HttpCodes} from '@actions/http-client' +import {BearerCredentialHandler} from '@actions/http-client/lib/auth' +import {info} from '@actions/core' +import {ArtifactServiceClientJSON} from '../../generated' +import {getResultsServiceUrl, getRuntimeToken} from './config' // The twirp http client must implement this interface interface Rpc { request( service: string, method: string, - contentType: "application/json" | "application/protobuf", + contentType: 'application/json' | 'application/protobuf', data: object | Uint8Array ): Promise } @@ -21,7 +21,12 @@ class ArtifactHttpClient implements Rpc { private baseRetryIntervalMilliseconds: number = 3000 private retryMultiplier: number = 1.5 - constructor(userAgent: string, maxAttempts?: number, baseRetryIntervalMilliseconds?: number, retryMultiplier?: number) { + constructor( + userAgent: string, + maxAttempts?: number, + baseRetryIntervalMilliseconds?: number, + retryMultiplier?: number + ) { const token = getRuntimeToken() this.baseUrl = getResultsServiceUrl() if (maxAttempts) { @@ -34,10 +39,9 @@ class ArtifactHttpClient implements Rpc { this.retryMultiplier = retryMultiplier } - 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 @@ -45,17 +49,19 @@ class ArtifactHttpClient implements Rpc { async request( service: string, method: string, - contentType: "application/json" | "application/protobuf", + contentType: 'application/json' | 'application/protobuf', data: object | Uint8Array ): Promise { let url = `${this.baseUrl}/twirp/${service}/${method}` let headers = { - "Content-Type": contentType, + 'Content-Type': contentType } info(`Making request to ${url} with data: ${JSON.stringify(data)}`) try { - const response = await this.retryableRequest(() => this.httpClient.post(url, JSON.stringify(data), headers)) + const response = await this.retryableRequest(() => + this.httpClient.post(url, JSON.stringify(data), headers) + ) const body = await response.readBody() return JSON.parse(body) } catch (error) { @@ -64,10 +70,10 @@ class ArtifactHttpClient implements Rpc { } async retryableRequest( - operation: () => Promise, + operation: () => Promise ): Promise { let attempt = 0 - let errorMessage = "" + let errorMessage = '' while (attempt < this.maxAttempts) { let isRetryable = false @@ -91,12 +97,17 @@ class ArtifactHttpClient implements Rpc { } if (attempt + 1 === this.maxAttempts) { - throw new Error(`Failed to make request after ${this.maxAttempts} attempts: ${errorMessage}`) + throw new Error( + `Failed to make request after ${this.maxAttempts} attempts: ${errorMessage}` + ) } - const retryTimeMilliseconds = this.getExponentialRetryTimeMilliseconds(attempt) + const retryTimeMilliseconds = + this.getExponentialRetryTimeMilliseconds(attempt) info( - `Attempt ${attempt + 1} of ${this.maxAttempts} failed with error: ${errorMessage}. Retrying request in ${retryTimeMilliseconds} ms...` + `Attempt ${attempt + 1} of ${ + this.maxAttempts + } failed with error: ${errorMessage}. Retrying request in ${retryTimeMilliseconds} ms...` ) await this.sleep(retryTimeMilliseconds) attempt++ @@ -131,14 +142,15 @@ class ArtifactHttpClient implements Rpc { getExponentialRetryTimeMilliseconds(attempt: number): number { if (attempt < 0) { - throw new Error("attempt should be a positive integer") + throw new Error('attempt should be a positive integer') } if (attempt === 0) { return this.baseRetryIntervalMilliseconds } - const minTime = this.baseRetryIntervalMilliseconds * this.retryMultiplier ** (attempt) + const minTime = + this.baseRetryIntervalMilliseconds * this.retryMultiplier ** attempt const maxTime = minTime * this.retryMultiplier // returns a random number between minTime and maxTime (exclusive) @@ -147,15 +159,15 @@ class ArtifactHttpClient implements Rpc { } export function createArtifactTwirpClient( - type: "upload" | "download", + type: 'upload' | 'download', maxAttempts?: number, baseRetryIntervalMilliseconds?: number, retryMultiplier?: number ): ArtifactServiceClientJSON { const client = new ArtifactHttpClient( - `@actions/artifact-${type}`, - maxAttempts, - baseRetryIntervalMilliseconds, + `@actions/artifact-${type}`, + maxAttempts, + baseRetryIntervalMilliseconds, retryMultiplier ) return new ArtifactServiceClientJSON(client) From cfad1451e9e2f82d3476c9b43388addd2a75418b Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 9 Aug 2023 07:00:27 -0700 Subject: [PATCH 11/15] Update generated files to not use bigint --- .../generated/google/protobuf/timestamp.ts | 18 ++++++------- .../src/generated/google/protobuf/wrappers.ts | 26 +++++++++---------- .../src/generated/results/api/v1/artifact.ts | 22 ++++++++-------- packages/artifact/tsconfig.json | 1 - 4 files changed, 33 insertions(+), 34 deletions(-) diff --git a/packages/artifact/src/generated/google/protobuf/timestamp.ts b/packages/artifact/src/generated/google/protobuf/timestamp.ts index b3b83738..c9aee41a 100644 --- a/packages/artifact/src/generated/google/protobuf/timestamp.ts +++ b/packages/artifact/src/generated/google/protobuf/timestamp.ts @@ -1,4 +1,4 @@ -// @generated by protobuf-ts 2.2.3-alpha.1 with parameter client_none,generate_dependencies +// @generated by protobuf-ts 2.9.1 with parameter long_type_string,client_none,generate_dependencies // @generated from protobuf file "google/protobuf/timestamp.proto" (package "google.protobuf", syntax proto3) // tslint:disable // @@ -139,7 +139,7 @@ export interface Timestamp { * * @generated from protobuf field: int64 seconds = 1; */ - seconds: bigint; + seconds: string; /** * Non-negative fractions of a second at nanosecond resolution. Negative * second values with fractions must still have non-negative nanos values @@ -154,7 +154,7 @@ export interface Timestamp { class Timestamp$Type extends MessageType { constructor() { super("google.protobuf.Timestamp", [ - { no: 1, name: "seconds", kind: "scalar", T: 3 /*ScalarType.INT64*/, L: 0 /*LongType.BIGINT*/ }, + { no: 1, name: "seconds", kind: "scalar", T: 3 /*ScalarType.INT64*/ }, { no: 2, name: "nanos", kind: "scalar", T: 5 /*ScalarType.INT32*/ } ]); } @@ -164,7 +164,7 @@ class Timestamp$Type extends MessageType { now(): Timestamp { const msg = this.create(); const ms = Date.now(); - msg.seconds = PbLong.from(Math.floor(ms / 1000)).toBigInt(); + msg.seconds = PbLong.from(Math.floor(ms / 1000)).toString(); msg.nanos = (ms % 1000) * 1000000; return msg; } @@ -180,7 +180,7 @@ class Timestamp$Type extends MessageType { fromDate(date: Date): Timestamp { const msg = this.create(); const ms = date.getTime(); - msg.seconds = PbLong.from(Math.floor(ms / 1000)).toBigInt(); + msg.seconds = PbLong.from(Math.floor(ms / 1000)).toString(); msg.nanos = (ms % 1000) * 1000000; return msg; } @@ -223,14 +223,14 @@ class Timestamp$Type extends MessageType { throw new globalThis.Error("Unable to parse Timestamp from JSON. Must be from 0001-01-01T00:00:00Z to 9999-12-31T23:59:59Z inclusive."); if (!target) target = this.create(); - target.seconds = PbLong.from(ms / 1000).toBigInt(); + target.seconds = PbLong.from(ms / 1000).toString(); target.nanos = 0; if (matches[7]) target.nanos = (parseInt("1" + matches[7] + "0".repeat(9 - matches[7].length)) - 1000000000); return target; } create(value?: PartialMessage): Timestamp { - const message = { seconds: 0n, nanos: 0 }; + const message = { seconds: "0", nanos: 0 }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -242,7 +242,7 @@ class Timestamp$Type extends MessageType { let [fieldNo, wireType] = reader.tag(); switch (fieldNo) { case /* int64 seconds */ 1: - message.seconds = reader.int64().toBigInt(); + message.seconds = reader.int64().toString(); break; case /* int32 nanos */ 2: message.nanos = reader.int32(); @@ -260,7 +260,7 @@ class Timestamp$Type extends MessageType { } internalBinaryWrite(message: Timestamp, writer: IBinaryWriter, options: BinaryWriteOptions): IBinaryWriter { /* int64 seconds = 1; */ - if (message.seconds !== 0n) + if (message.seconds !== "0") writer.tag(1, WireType.Varint).int64(message.seconds); /* int32 nanos = 2; */ if (message.nanos !== 0) diff --git a/packages/artifact/src/generated/google/protobuf/wrappers.ts b/packages/artifact/src/generated/google/protobuf/wrappers.ts index 83acb241..8f8797b0 100644 --- a/packages/artifact/src/generated/google/protobuf/wrappers.ts +++ b/packages/artifact/src/generated/google/protobuf/wrappers.ts @@ -1,4 +1,4 @@ -// @generated by protobuf-ts 2.2.3-alpha.1 with parameter client_none,generate_dependencies +// @generated by protobuf-ts 2.9.1 with parameter long_type_string,client_none,generate_dependencies // @generated from protobuf file "google/protobuf/wrappers.proto" (package "google.protobuf", syntax proto3) // tslint:disable // @@ -96,7 +96,7 @@ export interface Int64Value { * * @generated from protobuf field: int64 value = 1; */ - value: bigint; + value: string; } /** * Wrapper message for `uint64`. @@ -111,7 +111,7 @@ export interface UInt64Value { * * @generated from protobuf field: uint64 value = 1; */ - value: bigint; + value: string; } /** * Wrapper message for `int32`. @@ -316,7 +316,7 @@ export const FloatValue = new FloatValue$Type(); class Int64Value$Type extends MessageType { constructor() { super("google.protobuf.Int64Value", [ - { no: 1, name: "value", kind: "scalar", T: 3 /*ScalarType.INT64*/, L: 0 /*LongType.BIGINT*/ } + { no: 1, name: "value", kind: "scalar", T: 3 /*ScalarType.INT64*/ } ]); } /** @@ -331,11 +331,11 @@ class Int64Value$Type extends MessageType { internalJsonRead(json: JsonValue, options: JsonReadOptions, target?: Int64Value): Int64Value { if (!target) target = this.create(); - target.value = this.refJsonReader.scalar(json, ScalarType.INT64, LongType.BIGINT, "value") as any; + target.value = this.refJsonReader.scalar(json, ScalarType.INT64, LongType.STRING, "value") as any; return target; } create(value?: PartialMessage): Int64Value { - const message = { value: 0n }; + const message = { value: "0" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -347,7 +347,7 @@ class Int64Value$Type extends MessageType { let [fieldNo, wireType] = reader.tag(); switch (fieldNo) { case /* int64 value */ 1: - message.value = reader.int64().toBigInt(); + message.value = reader.int64().toString(); break; default: let u = options.readUnknownField; @@ -362,7 +362,7 @@ class Int64Value$Type extends MessageType { } internalBinaryWrite(message: Int64Value, writer: IBinaryWriter, options: BinaryWriteOptions): IBinaryWriter { /* int64 value = 1; */ - if (message.value !== 0n) + if (message.value !== "0") writer.tag(1, WireType.Varint).int64(message.value); let u = options.writeUnknownFields; if (u !== false) @@ -378,7 +378,7 @@ export const Int64Value = new Int64Value$Type(); class UInt64Value$Type extends MessageType { constructor() { super("google.protobuf.UInt64Value", [ - { no: 1, name: "value", kind: "scalar", T: 4 /*ScalarType.UINT64*/, L: 0 /*LongType.BIGINT*/ } + { no: 1, name: "value", kind: "scalar", T: 4 /*ScalarType.UINT64*/ } ]); } /** @@ -393,11 +393,11 @@ class UInt64Value$Type extends MessageType { internalJsonRead(json: JsonValue, options: JsonReadOptions, target?: UInt64Value): UInt64Value { if (!target) target = this.create(); - target.value = this.refJsonReader.scalar(json, ScalarType.UINT64, LongType.BIGINT, "value") as any; + target.value = this.refJsonReader.scalar(json, ScalarType.UINT64, LongType.STRING, "value") as any; return target; } create(value?: PartialMessage): UInt64Value { - const message = { value: 0n }; + const message = { value: "0" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -409,7 +409,7 @@ class UInt64Value$Type extends MessageType { let [fieldNo, wireType] = reader.tag(); switch (fieldNo) { case /* uint64 value */ 1: - message.value = reader.uint64().toBigInt(); + message.value = reader.uint64().toString(); break; default: let u = options.readUnknownField; @@ -424,7 +424,7 @@ class UInt64Value$Type extends MessageType { } internalBinaryWrite(message: UInt64Value, writer: IBinaryWriter, options: BinaryWriteOptions): IBinaryWriter { /* uint64 value = 1; */ - if (message.value !== 0n) + if (message.value !== "0") writer.tag(1, WireType.Varint).uint64(message.value); let u = options.writeUnknownFields; if (u !== false) diff --git a/packages/artifact/src/generated/results/api/v1/artifact.ts b/packages/artifact/src/generated/results/api/v1/artifact.ts index 89bae48d..106c4cda 100644 --- a/packages/artifact/src/generated/results/api/v1/artifact.ts +++ b/packages/artifact/src/generated/results/api/v1/artifact.ts @@ -1,4 +1,4 @@ -// @generated by protobuf-ts 2.2.3-alpha.1 with parameter client_none,generate_dependencies +// @generated by protobuf-ts 2.9.1 with parameter long_type_string,client_none,generate_dependencies // @generated from protobuf file "results/api/v1/artifact.proto" (package "github.actions.results.api.v1", syntax proto3) // tslint:disable import { ServiceType } from "@protobuf-ts/runtime-rpc"; @@ -71,7 +71,7 @@ export interface FinalizeArtifactRequest { /** * @generated from protobuf field: int64 size = 4; */ - size: bigint; + size: string; /** * @generated from protobuf field: google.protobuf.StringValue hash = 5; */ @@ -88,7 +88,7 @@ export interface FinalizeArtifactResponse { /** * @generated from protobuf field: int64 artifact_id = 2; */ - artifactId: bigint; + artifactId: string; } // @generated message type with reflection information, may provide speed optimized methods class CreateArtifactRequest$Type extends MessageType { @@ -226,12 +226,12 @@ class FinalizeArtifactRequest$Type extends MessageType { no: 1, name: "workflow_run_backend_id", kind: "scalar", T: 9 /*ScalarType.STRING*/ }, { no: 2, name: "workflow_job_run_backend_id", kind: "scalar", T: 9 /*ScalarType.STRING*/ }, { no: 3, name: "name", kind: "scalar", T: 9 /*ScalarType.STRING*/ }, - { no: 4, name: "size", kind: "scalar", T: 3 /*ScalarType.INT64*/, L: 0 /*LongType.BIGINT*/ }, + { no: 4, name: "size", kind: "scalar", T: 3 /*ScalarType.INT64*/ }, { no: 5, name: "hash", kind: "message", T: () => StringValue } ]); } create(value?: PartialMessage): FinalizeArtifactRequest { - const message = { workflowRunBackendId: "", workflowJobRunBackendId: "", name: "", size: 0n }; + const message = { workflowRunBackendId: "", workflowJobRunBackendId: "", name: "", size: "0" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -252,7 +252,7 @@ class FinalizeArtifactRequest$Type extends MessageType message.name = reader.string(); break; case /* int64 size */ 4: - message.size = reader.int64().toBigInt(); + message.size = reader.int64().toString(); break; case /* google.protobuf.StringValue hash */ 5: message.hash = StringValue.internalBinaryRead(reader, reader.uint32(), options, message.hash); @@ -279,7 +279,7 @@ class FinalizeArtifactRequest$Type extends MessageType if (message.name !== "") writer.tag(3, WireType.LengthDelimited).string(message.name); /* int64 size = 4; */ - if (message.size !== 0n) + if (message.size !== "0") writer.tag(4, WireType.Varint).int64(message.size); /* google.protobuf.StringValue hash = 5; */ if (message.hash) @@ -299,11 +299,11 @@ class FinalizeArtifactResponse$Type extends MessageType): FinalizeArtifactResponse { - const message = { ok: false, artifactId: 0n }; + const message = { ok: false, artifactId: "0" }; globalThis.Object.defineProperty(message, MESSAGE_TYPE, { enumerable: false, value: this }); if (value !== undefined) reflectionMergePartial(this, message, value); @@ -318,7 +318,7 @@ class FinalizeArtifactResponse$Type extends MessageType Date: Wed, 9 Aug 2023 07:02:06 -0700 Subject: [PATCH 12/15] don't add extra line breaks --- package.json | 2 +- tsconfig.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 20221837..d64d2841 100644 --- a/package.json +++ b/package.json @@ -33,4 +33,4 @@ "ts-jest": "^27.0.5", "typescript": "^3.9.9" } -} +} \ No newline at end of file diff --git a/tsconfig.json b/tsconfig.json index 14c7f781..fb610c6e 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -22,4 +22,4 @@ "**/*.test.ts", "**/__mocks__/*" ] -} +} \ No newline at end of file From 24da3e2d1c641dad766dc02a22079e408e49dcb4 Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 9 Aug 2023 07:10:43 -0700 Subject: [PATCH 13/15] lint --- .../artifact/__tests__/artifact-http-client.test.ts | 10 +++++----- .../src/internal/shared/artifact-twirp-client.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/artifact/__tests__/artifact-http-client.test.ts b/packages/artifact/__tests__/artifact-http-client.test.ts index 91327114..37908111 100644 --- a/packages/artifact/__tests__/artifact-http-client.test.ts +++ b/packages/artifact/__tests__/artifact-http-client.test.ts @@ -35,7 +35,7 @@ describe('artifact-http-client', () => { msg.statusCode = 200 return { message: msg, - readBody: () => { + readBody: async () => { return Promise.resolve( `{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}` ) @@ -72,7 +72,7 @@ describe('artifact-http-client', () => { msgSucceeded.statusCode = 200 return { message: msgSucceeded, - readBody: () => { + readBody: async () => { return Promise.resolve( `{"ok": true, "signedUploadUrl": "http://localhost:8080/upload"}` ) @@ -85,7 +85,7 @@ describe('artifact-http-client', () => { msgFailed.statusMessage = 'Internal Server Error' return { message: msgFailed, - readBody: () => { + readBody: async () => { return Promise.resolve(`{"ok": false}`) } } @@ -125,7 +125,7 @@ describe('artifact-http-client', () => { msgFailed.statusMessage = 'Internal Server Error' return { message: msgFailed, - readBody: () => { + readBody: async () => { return Promise.resolve(`{"ok": false}`) } } @@ -165,7 +165,7 @@ describe('artifact-http-client', () => { msgFailed.statusMessage = 'Unauthorized' return { message: msgFailed, - readBody: () => { + readBody: async () => { return Promise.resolve(`{"ok": false}`) } } diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index e582db45..d2450183 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -17,9 +17,9 @@ interface Rpc { class ArtifactHttpClient implements Rpc { private httpClient: HttpClient private baseUrl: string - private maxAttempts: number = 5 - private baseRetryIntervalMilliseconds: number = 3000 - private retryMultiplier: number = 1.5 + private maxAttempts = 5 + private baseRetryIntervalMilliseconds = 3000 + private retryMultiplier = 1.5 constructor( userAgent: string, @@ -52,14 +52,14 @@ class ArtifactHttpClient implements Rpc { contentType: 'application/json' | 'application/protobuf', data: object | Uint8Array ): Promise { - let url = `${this.baseUrl}/twirp/${service}/${method}` - let headers = { + const url = `${this.baseUrl}/twirp/${service}/${method}` + const headers = { 'Content-Type': contentType } info(`Making request to ${url} with data: ${JSON.stringify(data)}`) try { - const response = await this.retryableRequest(() => + const response = await this.retryableRequest(async () => this.httpClient.post(url, JSON.stringify(data), headers) ) const body = await response.readBody() From c6117995d3604525bd93f93d7c1a5d49dfded8e5 Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 9 Aug 2023 10:30:44 -0400 Subject: [PATCH 14/15] Update packages/artifact/src/internal/shared/config.ts Co-authored-by: Konrad Pabjan --- packages/artifact/src/internal/shared/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/src/internal/shared/config.ts b/packages/artifact/src/internal/shared/config.ts index e0d41ba5..c4e5f4a1 100644 --- a/packages/artifact/src/internal/shared/config.ts +++ b/packages/artifact/src/internal/shared/config.ts @@ -1,7 +1,7 @@ export function getRuntimeToken(): string { const token = process.env['ACTIONS_RUNTIME_TOKEN'] if (!token) { - throw new Error('Unable to get ACTIONS_RUNTIME_TOKEN env variable') + throw new Error('Unable to get the ACTIONS_RUNTIME_TOKEN env variable') } return token } From 760f3fd3d1619a47c8ac9299dd1f581078865048 Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 9 Aug 2023 10:30:50 -0400 Subject: [PATCH 15/15] Update packages/artifact/src/internal/shared/config.ts Co-authored-by: Konrad Pabjan --- packages/artifact/src/internal/shared/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/src/internal/shared/config.ts b/packages/artifact/src/internal/shared/config.ts index c4e5f4a1..23949f20 100644 --- a/packages/artifact/src/internal/shared/config.ts +++ b/packages/artifact/src/internal/shared/config.ts @@ -9,7 +9,7 @@ export function getRuntimeToken(): string { export function getResultsServiceUrl(): string { const resultsUrl = process.env['ACTIONS_RESULTS_URL'] if (!resultsUrl) { - throw new Error('Unable to get ACTIONS_RESULTS_URL env variable') + throw new Error('Unable to get the ACTIONS_RESULTS_URL env variable') } return resultsUrl }