From 898dd8c2a1770dd666e0f3513f86572c47cbd15d Mon Sep 17 00:00:00 2001 From: Bethany Date: Wed, 2 Aug 2023 12:03:41 -0700 Subject: [PATCH] Add twirp client with retry logic --- packages/artifact/package-lock.json | 13 +- packages/artifact/package.json | 1 + .../generated/google/protobuf/timestamp.ts | 2 +- .../generated/google/protobuf/wrappers.ts | 2 +- packages/artifact/src/generated/index.ts | 4 + .../generated/results/api/v1/artifact.ts | 2 +- .../results/api/v1/artifact.twirp.ts | 0 .../src/internal/artifact-http-client.ts | 139 ++++++++++++++++++ packages/artifact/src/internal/config.ts | 17 ++- .../src/internal/upload/tmp/twirp_testing.ts | 32 ++++ packages/artifact/tsconfig.json | 9 +- 11 files changed, 209 insertions(+), 12 deletions(-) rename packages/artifact/{ => src}/generated/google/protobuf/timestamp.ts (99%) rename packages/artifact/{ => src}/generated/google/protobuf/wrappers.ts (99%) create mode 100644 packages/artifact/src/generated/index.ts rename packages/artifact/{ => src}/generated/results/api/v1/artifact.ts (99%) rename packages/artifact/{ => src}/generated/results/api/v1/artifact.twirp.ts (100%) create mode 100644 packages/artifact/src/internal/artifact-http-client.ts create mode 100644 packages/artifact/src/internal/upload/tmp/twirp_testing.ts diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index ea71c924..396d3751 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@actions/core": "^1.10.0", + "@actions/http-client": "^2.1.0", "@azure/storage-blob": "^12.15.0", "@types/node": "^20.4.5", "archiver": "^5.3.1" @@ -31,9 +32,9 @@ } }, "node_modules/@actions/http-client": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.0.1.tgz", - "integrity": "sha512-PIXiMVtz6VvyaRsGY268qvj57hXQEpsYogYOu2nrQhlf+XCGmZstmuZBbAybUl1nQGnvS1k1eEsQ69ZoD7xlSw==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.1.0.tgz", + "integrity": "sha512-BonhODnXr3amchh4qkmjPMUO8mFi/zLaaCeCAJZqch8iQqyDnVIkySjB38VHAC8IJ+bnlgfOqlhpyCUZHlQsqw==", "dependencies": { "tunnel": "^0.0.6" } @@ -1070,9 +1071,9 @@ } }, "@actions/http-client": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.0.1.tgz", - "integrity": "sha512-PIXiMVtz6VvyaRsGY268qvj57hXQEpsYogYOu2nrQhlf+XCGmZstmuZBbAybUl1nQGnvS1k1eEsQ69ZoD7xlSw==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.1.0.tgz", + "integrity": "sha512-BonhODnXr3amchh4qkmjPMUO8mFi/zLaaCeCAJZqch8iQqyDnVIkySjB38VHAC8IJ+bnlgfOqlhpyCUZHlQsqw==", "requires": { "tunnel": "^0.0.6" } diff --git a/packages/artifact/package.json b/packages/artifact/package.json index 9836dcd2..4d38df83 100644 --- a/packages/artifact/package.json +++ b/packages/artifact/package.json @@ -38,6 +38,7 @@ }, "dependencies": { "@actions/core": "^1.10.0", + "@actions/http-client": "^2.1.0", "@azure/storage-blob": "^12.15.0", "@types/node": "^20.4.5", "archiver": "^5.3.1" diff --git a/packages/artifact/generated/google/protobuf/timestamp.ts b/packages/artifact/src/generated/google/protobuf/timestamp.ts similarity index 99% rename from packages/artifact/generated/google/protobuf/timestamp.ts rename to packages/artifact/src/generated/google/protobuf/timestamp.ts index b3b83738..55d10e0d 100644 --- a/packages/artifact/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 client_none,generate_dependencies // @generated from protobuf file "google/protobuf/timestamp.proto" (package "google.protobuf", syntax proto3) // tslint:disable // diff --git a/packages/artifact/generated/google/protobuf/wrappers.ts b/packages/artifact/src/generated/google/protobuf/wrappers.ts similarity index 99% rename from packages/artifact/generated/google/protobuf/wrappers.ts rename to packages/artifact/src/generated/google/protobuf/wrappers.ts index 83acb241..bd7c2aed 100644 --- a/packages/artifact/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 client_none,generate_dependencies // @generated from protobuf file "google/protobuf/wrappers.proto" (package "google.protobuf", syntax proto3) // tslint:disable // diff --git a/packages/artifact/src/generated/index.ts b/packages/artifact/src/generated/index.ts new file mode 100644 index 00000000..49ca3acf --- /dev/null +++ b/packages/artifact/src/generated/index.ts @@ -0,0 +1,4 @@ +export * from "../generated/google/protobuf/timestamp"; +export * from "../generated/google/protobuf/wrappers"; +export * from "../generated/results/api/v1/artifact"; +export * from "../generated/results/api/v1/artifact.twirp"; diff --git a/packages/artifact/generated/results/api/v1/artifact.ts b/packages/artifact/src/generated/results/api/v1/artifact.ts similarity index 99% rename from packages/artifact/generated/results/api/v1/artifact.ts rename to packages/artifact/src/generated/results/api/v1/artifact.ts index ccff08c1..05697fbd 100644 --- a/packages/artifact/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 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"; diff --git a/packages/artifact/generated/results/api/v1/artifact.twirp.ts b/packages/artifact/src/generated/results/api/v1/artifact.twirp.ts similarity index 100% rename from packages/artifact/generated/results/api/v1/artifact.twirp.ts rename to packages/artifact/src/generated/results/api/v1/artifact.twirp.ts diff --git a/packages/artifact/src/internal/artifact-http-client.ts b/packages/artifact/src/internal/artifact-http-client.ts new file mode 100644 index 00000000..d69b0d01 --- /dev/null +++ b/packages/artifact/src/internal/artifact-http-client.ts @@ -0,0 +1,139 @@ +import { HttpCodes, HttpClient, HttpClientResponse } from '@actions/http-client' +import { BearerCredentialHandler } from '@actions/http-client/lib/auth' +import { info } from '@actions/core' +import { getRuntimeToken, getResultsServiceUrl, getRetryMultiplier, getInitialRetryIntervalInMilliseconds, getRetryLimit } from './config' + +interface Rpc { request( + service: string, + method: string, + contentType: "application/json" | "application/protobuf", + data: object | Uint8Array + ): Promise +} + +export class ArtifactHttpClient implements Rpc { + private httpClient: HttpClient + private baseUrl: string + + constructor(userAgent: string) { + this.httpClient = new HttpClient(userAgent, [ + new BearerCredentialHandler(getRuntimeToken()) + ]) + this.baseUrl = getResultsServiceUrl() + } + + 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 resp = await this.retry( + `${method}`, + this.httpClient.post(url, JSON.stringify(data), headers), + ) + const body = await resp.readBody() + return JSON.parse(body) + } + + async retry(name: string, operation: Promise): Promise { + let response: HttpClientResponse | undefined = undefined + let statusCode: number | undefined = undefined + let isRetryable = false + let errorMessage = '' + let attempt = 1 + const maxAttempts = getRetryLimit() + + while (attempt <= maxAttempts) { + try { + response = await operation + statusCode = response.message.statusCode + + if (this.isSuccessStatusCode(statusCode)) { + return response + } + + isRetryable = this.isRetryableStatusCode(statusCode) + errorMessage = `Artifact service responded with ${statusCode}` + } catch (error) { + isRetryable = true + errorMessage = error.message + } + + if (!isRetryable) { + info(`${name} - Error is not retryable`) + if (response) { + this.displayHttpDiagnostics(response) + } + break + } + + info( + `${name} - Attempt ${attempt} of ${maxAttempts} failed with error: ${errorMessage}` + ) + + await this.sleep(this.getExponentialRetryTimeInMilliseconds(attempt)) + attempt++ + } + + if (response) { + this.displayHttpDiagnostics(response) + } + + throw Error(`${name} failed: ${errorMessage}`) + } + + isSuccessStatusCode(statusCode?: number): boolean { + if (!statusCode) { + return false + } + return statusCode >= 200 && statusCode < 300 + } + + isRetryableStatusCode(statusCode: number | undefined): 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) + } + + displayHttpDiagnostics(response: HttpClientResponse): void { + info( + `##### Begin Diagnostic HTTP information ##### + Status Code: ${response.message.statusCode} + Status Message: ${response.message.statusMessage} + Header Information: ${JSON.stringify(response.message.headers, undefined, 2)} + ###### End Diagnostic HTTP information ######` + ) + } + + getExponentialRetryTimeInMilliseconds( + retryCount: number + ): number { + if (retryCount < 0) { + throw new Error('RetryCount should not be negative') + } else if (retryCount === 0) { + return getInitialRetryIntervalInMilliseconds() + } + + const minTime = + getInitialRetryIntervalInMilliseconds() * getRetryMultiplier() * retryCount + const maxTime = minTime * getRetryMultiplier() + + // returns a random number between the minTime (inclusive) and the maxTime (exclusive) + return Math.trunc(Math.random() * (maxTime - minTime) + minTime) + } + + async sleep(milliseconds: number): Promise { + return new Promise(resolve => setTimeout(resolve, milliseconds)) + } +} diff --git a/packages/artifact/src/internal/config.ts b/packages/artifact/src/internal/config.ts index c17aeb44..cd95e34d 100644 --- a/packages/artifact/src/internal/config.ts +++ b/packages/artifact/src/internal/config.ts @@ -32,4 +32,19 @@ export function getWorkSpaceDirectory(): string { export function getRetentionDays(): string | undefined { return process.env['GITHUB_RETENTION_DAYS'] -} \ No newline at end of file +} + +export function getInitialRetryIntervalInMilliseconds(): number { + return 3000 +} + +// With exponential backoff, the larger the retry count, the larger the wait time before another attempt +// The retry multiplier controls by how much the backOff time increases depending on the number of retries +export function getRetryMultiplier(): number { + return 1.5 +} + +// The maximum number of retries that can be attempted before an upload or download fails +export function getRetryLimit(): number { + return 5 +} diff --git a/packages/artifact/src/internal/upload/tmp/twirp_testing.ts b/packages/artifact/src/internal/upload/tmp/twirp_testing.ts new file mode 100644 index 00000000..ada7303b --- /dev/null +++ b/packages/artifact/src/internal/upload/tmp/twirp_testing.ts @@ -0,0 +1,32 @@ +import { ArtifactHttpClient } from '../../artifact-http-client' +import { ArtifactServiceClientJSON } from '../../../generated' + +export async function twirpTest(){ + const artifactClient = new ArtifactHttpClient('@actions/artifact-upload') + const jsonClient = new ArtifactServiceClientJSON(artifactClient) + + try { + const createResp = await jsonClient.CreateArtifact({workflowRunBackendId: "ce7f54c7-61c7-4aae-887f-30da475f5f1a", workflowJobRunBackendId: "ca395085-040a-526b-2ce8-bdc85f692774", name: Math.random().toString(), version: 4}) + + if (!createResp.ok) { + console.log("CreateArtifact failed") + return + } + + console.log(createResp.signedUploadUrl) + + const finalizeResp = await jsonClient.FinalizeArtifact({workflowRunBackendId: "ce7f54c7-61c7-4aae-887f-30da475f5f1a", workflowJobRunBackendId: "ca395085-040a-526b-2ce8-bdc85f692774", name: Math.random().toString(), size: BigInt(5)}) + + if (!finalizeResp.ok) { + console.log("FinalizeArtifact failed") + return + } + } catch (e) { + console.log(e) + return + } + + console.log("FinalizeArtifact succeeded") +} + +twirpTest() diff --git a/packages/artifact/tsconfig.json b/packages/artifact/tsconfig.json index a8b812a6..a821322f 100644 --- a/packages/artifact/tsconfig.json +++ b/packages/artifact/tsconfig.json @@ -3,9 +3,14 @@ "compilerOptions": { "baseUrl": "./", "outDir": "./lib", - "rootDir": "./src" + "rootDir": "./src", + "lib": [ + "es2020" + ], + "module": "commonjs", + "target": "es2020" }, "include": [ "./src" ] -} \ No newline at end of file +}