From 571bf222ee54f2a9d46a7702cd83aedfbdfb2f02 Mon Sep 17 00:00:00 2001 From: srryan Date: Mon, 18 Dec 2023 17:11:14 -0500 Subject: [PATCH 01/31] update to use blob client over http client --- .../src/internal/download/download-artifact.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index a61e2937..00e13853 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -17,6 +17,8 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' +import {BlobClient, } from '@azure/storage-blob' + const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -38,18 +40,11 @@ async function exists(path: string): Promise { } async function streamExtract(url: string, directory: string): Promise { - const client = new httpClient.HttpClient(getUserAgentString()) - const response = await client.get(url) - - if (response.message.statusCode !== 200) { - throw new Error( - `Unexpected HTTP response from blob storage: ${response.message.statusCode} ${response.message.statusMessage}` - ) - } - + const blobClient = new BlobClient(url) + const response = await blobClient.download() + return new Promise((resolve, reject) => { - response.message - .pipe(unzip.Extract({path: directory})) + response.readableStreamBody?.pipe(unzip.Extract({path: directory})) .on('close', resolve) .on('error', reject) }) From 0c0770ce572bccd22a1cc9579dd0dff0eb2904bb Mon Sep 17 00:00:00 2001 From: srryan Date: Mon, 18 Dec 2023 17:52:55 -0500 Subject: [PATCH 02/31] cleanup --- packages/artifact/src/internal/download/download-artifact.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 00e13853..d4900c92 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -17,7 +17,7 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' -import {BlobClient, } from '@azure/storage-blob' +import {BlobClient} from '@azure/storage-blob' const scrubQueryParameters = (url: string): string => { From bf93b54558064d6af5c024299b75e8f7ad2c7704 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Mon, 18 Dec 2023 23:09:10 +0000 Subject: [PATCH 03/31] adding logger for blob client and response --- packages/artifact/package-lock.json | 237 +++++++++++++++++- packages/artifact/package.json | 3 +- .../internal/download/download-artifact.ts | 12 +- 3 files changed, 245 insertions(+), 7 deletions(-) diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index abe007be..aa86d4d9 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -22,7 +22,8 @@ "crypto": "^1.0.1", "jwt-decode": "^3.1.2", "twirp-ts": "^2.5.0", - "unzip-stream": "^0.3.1" + "unzip-stream": "^0.3.1", + "util": "^0.12.5" }, "devDependencies": { "@types/archiver": "^5.3.2", @@ -560,6 +561,17 @@ "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==" }, + "node_modules/available-typed-arrays": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/available-typed-arrays/-/available-typed-arrays-1.0.5.tgz", + "integrity": "sha512-DMD0KiN46eipeziST1LPP/STfDU0sufISXmjSgvVsoU2tqxctQeASejWcfNtxYKqETM1UxQ8sp2OrSBWpHY6sw==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -664,6 +676,19 @@ "node": ">=0.2.0" } }, + "node_modules/call-bind": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/call-bind/-/call-bind-1.0.5.tgz", + "integrity": "sha512-C3nQxfFZxFRVoJoGKKI8y3MOEo129NQ+FgQ08iye+Mk4zNZZGdjfs06bVTr+DBSlA66Q2VEcMki/cUCP4SercQ==", + "dependencies": { + "function-bind": "^1.1.2", + "get-intrinsic": "^1.2.1", + "set-function-length": "^1.1.1" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/camel-case": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/camel-case/-/camel-case-4.1.2.tgz", @@ -756,6 +781,19 @@ "integrity": "sha512-VxBKmeNcqQdiUQUW2Tzq0t377b54N2bMtXO/qiLa+6eRRmmC4qT3D4OnTGoT/U6O9aklQ/jTwbOtRMTTY8G0Ig==", "deprecated": "This package is no longer supported. It's now a built-in Node module. If you've depended on crypto, you should switch to the one that's built-in." }, + "node_modules/define-data-property": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/define-data-property/-/define-data-property-1.1.1.tgz", + "integrity": "sha512-E7uGkTzkk1d0ByLeSc6ZsFS79Axg+m1P/VsgYsxHgiuc3tFSj+MjMIwe90FC4lOAZzNBdY7kkO2P2wKdsQ1vgQ==", + "dependencies": { + "get-intrinsic": "^1.2.1", + "gopd": "^1.0.1", + "has-property-descriptors": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/delayed-stream": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", @@ -797,6 +835,14 @@ "node": ">=0.8.x" } }, + "node_modules/for-each": { + "version": "0.3.3", + "resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.3.tgz", + "integrity": "sha512-jqYfLp7mo9vIyQf8ykW2v7A+2N4QjeCeI5+Dz9XraiO1ign81wjiH7Fb9vSOWvQfNtmSa4H2RoQTrrXivdUZmw==", + "dependencies": { + "is-callable": "^1.1.3" + } + }, "node_modules/form-data": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", @@ -820,6 +866,28 @@ "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", "integrity": "sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==" }, + "node_modules/function-bind": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz", + "integrity": "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA==", + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/get-intrinsic": { + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/get-intrinsic/-/get-intrinsic-1.2.2.tgz", + "integrity": "sha512-0gSo4ml/0j98Y3lngkFEot/zhiCeWsbYIlZ+uZOVgzLyLaUw7wxUL+nCTP0XJvJg1AXulJRI3UJi8GsbDuxdGA==", + "dependencies": { + "function-bind": "^1.1.2", + "has-proto": "^1.0.1", + "has-symbols": "^1.0.3", + "hasown": "^2.0.0" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/glob": { "version": "7.2.3", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", @@ -839,6 +907,17 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/gopd": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.0.1.tgz", + "integrity": "sha512-d65bNlIadxvpb/A2abVdlqKqV563juRnZ1Wtk6s1sIR8uNsXR70xqIzVqxVf1eTqDunwT2MkczEeaezCKTZhwA==", + "dependencies": { + "get-intrinsic": "^1.1.3" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/graceful-fs": { "version": "4.2.11", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", @@ -865,6 +944,64 @@ "uglify-js": "^3.1.4" } }, + "node_modules/has-property-descriptors": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/has-property-descriptors/-/has-property-descriptors-1.0.1.tgz", + "integrity": "sha512-VsX8eaIewvas0xnvinAe9bw4WfIeODpGYikiWYLH+dma0Jw6KHYqWiWfhQlgOVK8D6PvjubK5Uc4P0iIhIcNVg==", + "dependencies": { + "get-intrinsic": "^1.2.2" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/has-proto": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/has-proto/-/has-proto-1.0.1.tgz", + "integrity": "sha512-7qE+iP+O+bgF9clE5+UoBFzE65mlBiVj3tKCrlNQ0Ogwm0BjpT/gK4SlLYDMybDh5I3TCTKnPPa0oMG7JDYrhg==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/has-symbols": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/has-symbols/-/has-symbols-1.0.3.tgz", + "integrity": "sha512-l3LCuF6MgDNwTDKkdYGEihYjt5pRPbEg46rtlmnSPlUbgmB8LOIrKJbYYFBSbnPaJexMKtiPO8hmeRjRz2Td+A==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/has-tostringtag": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/has-tostringtag/-/has-tostringtag-1.0.0.tgz", + "integrity": "sha512-kFjcSNhnlGV1kyoGk7OXKSawH5JOb/LzUc5w9B02hOTO0dfFRjbHQKvg1d6cf3HbeUmtU9VbbV3qzZ2Teh97WQ==", + "dependencies": { + "has-symbols": "^1.0.2" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/hasown": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.0.tgz", + "integrity": "sha512-vUptKVTpIJhcczKBbgnS+RtcuYMB8+oNzPK2/Hp3hanz8JmpATdmmgLgSaadVREkDm+e2giHwY3ZRkyjSIDDFA==", + "dependencies": { + "function-bind": "^1.1.2" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/ieee754": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", @@ -898,6 +1035,46 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, + "node_modules/is-arguments": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/is-arguments/-/is-arguments-1.1.1.tgz", + "integrity": "sha512-8Q7EARjzEnKpt/PCD7e1cgUS0a6X8u5tdSiMqXhojOdoV9TsMsiO+9VLC5vAmO8N7/GmXn7yjR8qnA6bVAEzfA==", + "dependencies": { + "call-bind": "^1.0.2", + "has-tostringtag": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/is-callable": { + "version": "1.2.7", + "resolved": "https://registry.npmjs.org/is-callable/-/is-callable-1.2.7.tgz", + "integrity": "sha512-1BC0BVFhS/p0qtw6enp8e+8OD0UrK0oFLztSjNzhcKA3WDuJxxAPXzPuPtKkjEY9UUoEWlX/8fgKeu2S8i9JTA==", + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, + "node_modules/is-generator-function": { + "version": "1.0.10", + "resolved": "https://registry.npmjs.org/is-generator-function/-/is-generator-function-1.0.10.tgz", + "integrity": "sha512-jsEjy9l3yiXEQ+PsXdmBwEPcOxaXWLspKdplFUVI9vq1iZgIekeC0L167qeu86czQaxed3q/Uzuw0swL0irL8A==", + "dependencies": { + "has-tostringtag": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/is-plain-object": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/is-plain-object/-/is-plain-object-5.0.0.tgz", @@ -906,6 +1083,20 @@ "node": ">=0.10.0" } }, + "node_modules/is-typed-array": { + "version": "1.1.12", + "resolved": "https://registry.npmjs.org/is-typed-array/-/is-typed-array-1.1.12.tgz", + "integrity": "sha512-Z14TF2JNG8Lss5/HMqt0//T9JeHXttXy5pH/DBU4vi98ozO2btxzq9MwYDZYnKwU8nRsz/+GVFVRDq3DkVuSPg==", + "dependencies": { + "which-typed-array": "^1.1.11" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/isarray": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz", @@ -1228,6 +1419,20 @@ "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" }, + "node_modules/set-function-length": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.1.1.tgz", + "integrity": "sha512-VoaqjbBJKiWtg4yRcKBQ7g7wnGnLV3M8oLvVWwOk2PdYY6PEFegR1vezXR0tw6fZGF9csVakIRjrJiy2veSBFQ==", + "dependencies": { + "define-data-property": "^1.1.1", + "get-intrinsic": "^1.2.1", + "gopd": "^1.0.1", + "has-property-descriptors": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + } + }, "node_modules/shiki": { "version": "0.14.5", "resolved": "https://registry.npmjs.org/shiki/-/shiki-0.14.5.tgz", @@ -1432,6 +1637,18 @@ "mkdirp": "^0.5.1" } }, + "node_modules/util": { + "version": "0.12.5", + "resolved": "https://registry.npmjs.org/util/-/util-0.12.5.tgz", + "integrity": "sha512-kZf/K6hEIrWHI6XqOFUiiMa+79wE/D8Q+NCNAWclkyg3b4d2k7s0QGepNjiABc+aR3N1PAyHL7p6UcLY6LmrnA==", + "dependencies": { + "inherits": "^2.0.3", + "is-arguments": "^1.0.4", + "is-generator-function": "^1.0.7", + "is-typed-array": "^1.1.3", + "which-typed-array": "^1.1.2" + } + }, "node_modules/util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", @@ -1471,6 +1688,24 @@ "webidl-conversions": "^3.0.0" } }, + "node_modules/which-typed-array": { + "version": "1.1.13", + "resolved": "https://registry.npmjs.org/which-typed-array/-/which-typed-array-1.1.13.tgz", + "integrity": "sha512-P5Nra0qjSncduVPEAr7xhoF5guty49ArDTwzJ/yNuPIbZppyRxFQsRCWrocxIY+CnMVG+qfbU2FmDKyvSGClow==", + "dependencies": { + "available-typed-arrays": "^1.0.5", + "call-bind": "^1.0.4", + "for-each": "^0.3.3", + "gopd": "^1.0.1", + "has-tostringtag": "^1.0.0" + }, + "engines": { + "node": ">= 0.4" + }, + "funding": { + "url": "https://github.com/sponsors/ljharb" + } + }, "node_modules/wordwrap": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-1.0.0.tgz", diff --git a/packages/artifact/package.json b/packages/artifact/package.json index c4f826c6..6cdde611 100644 --- a/packages/artifact/package.json +++ b/packages/artifact/package.json @@ -53,7 +53,8 @@ "crypto": "^1.0.1", "jwt-decode": "^3.1.2", "twirp-ts": "^2.5.0", - "unzip-stream": "^0.3.1" + "unzip-stream": "^0.3.1", + "util": "^0.12.5" }, "devDependencies": { "@types/archiver": "^5.3.2", diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index d4900c92..7b67454d 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -1,13 +1,14 @@ import fs from 'fs/promises' import * as github from '@actions/github' import * as core from '@actions/core' -import * as httpClient from '@actions/http-client' +// import * as httpClient from '@actions/http-client' +import * as util from 'util' import unzip from 'unzip-stream' import { DownloadArtifactOptions, DownloadArtifactResponse } from '../shared/interfaces' -import {getUserAgentString} from '../shared/user-agent' +// import {getUserAgentString} from '../shared/user-agent' import {getGitHubWorkspaceDir} from '../shared/config' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' import { @@ -19,7 +20,6 @@ import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' import {BlobClient} from '@azure/storage-blob' - const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) parsed.search = '' @@ -42,9 +42,11 @@ async function exists(path: string): Promise { async function streamExtract(url: string, directory: string): Promise { const blobClient = new BlobClient(url) const response = await blobClient.download() - + core.info(util.inspect(blobClient)) + core.info(util.inspect(response)) return new Promise((resolve, reject) => { - response.readableStreamBody?.pipe(unzip.Extract({path: directory})) + response.readableStreamBody + ?.pipe(unzip.Extract({path: directory})) .on('close', resolve) .on('error', reject) }) From 73babeabefd68ff5bd922a4507d757c0b80d2a3d Mon Sep 17 00:00:00 2001 From: srryan Date: Tue, 19 Dec 2023 11:49:39 -0500 Subject: [PATCH 04/31] add explicit options --- .../src/internal/download/download-artifact.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 7b67454d..443eb47b 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -18,7 +18,7 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' -import {BlobClient} from '@azure/storage-blob' +import {BlobClient, BlobDownloadOptions} from '@azure/storage-blob' const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -41,14 +41,20 @@ async function exists(path: string): Promise { async function streamExtract(url: string, directory: string): Promise { const blobClient = new BlobClient(url) - const response = await blobClient.download() - core.info(util.inspect(blobClient)) - core.info(util.inspect(response)) + const options: BlobDownloadOptions = { + maxRetryRequests: 3, + abortSignal: undefined, + onProgress: (progress) => core.debug(`Download progress: ${progress.loadedBytes}`) + } + + const response = await blobClient.download(0, 0, options) + return new Promise((resolve, reject) => { response.readableStreamBody ?.pipe(unzip.Extract({path: directory})) .on('close', resolve) .on('error', reject) + .on('aborted', reject) }) } From c119fcd773dc830a2310599e790da7efc4364749 Mon Sep 17 00:00:00 2001 From: srryan Date: Tue, 19 Dec 2023 12:02:10 -0500 Subject: [PATCH 05/31] update optional settings for blob client --- .../artifact/src/internal/download/download-artifact.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 443eb47b..bc64d51d 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -42,12 +42,11 @@ async function exists(path: string): Promise { async function streamExtract(url: string, directory: string): Promise { const blobClient = new BlobClient(url) const options: BlobDownloadOptions = { - maxRetryRequests: 3, - abortSignal: undefined, - onProgress: (progress) => core.debug(`Download progress: ${progress.loadedBytes}`) + maxRetryRequests: 5, + onProgress: (progress) => core.info(`Download bytes ${progress.loadedBytes}`) } - const response = await blobClient.download(0, 0, options) + const response = await blobClient.download(0, undefined, options) return new Promise((resolve, reject) => { response.readableStreamBody From 78ed49ff8807a9c7aee2376c68719f799afbeb6b Mon Sep 17 00:00:00 2001 From: srryan Date: Tue, 19 Dec 2023 12:46:58 -0500 Subject: [PATCH 06/31] update error handling abort --- .../artifact/src/internal/download/download-artifact.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index bc64d51d..6ff72a02 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -50,10 +50,11 @@ async function streamExtract(url: string, directory: string): Promise { return new Promise((resolve, reject) => { response.readableStreamBody - ?.pipe(unzip.Extract({path: directory})) - .on('close', resolve) - .on('error', reject) + ?.on('error', reject) .on('aborted', reject) + .pipe(unzip.Extract({path: directory})) + .on('error', reject) + .on('close', resolve) }) } From 2d6ba67518bb8d4a1a0974a591804b3457bd5eae Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Wed, 20 Dec 2023 13:11:04 -0500 Subject: [PATCH 07/31] retry the promise --- packages/artifact/package-lock.json | 237 +----------------- packages/artifact/package.json | 3 +- .../internal/download/download-artifact.ts | 48 ++-- 3 files changed, 35 insertions(+), 253 deletions(-) diff --git a/packages/artifact/package-lock.json b/packages/artifact/package-lock.json index aa86d4d9..abe007be 100644 --- a/packages/artifact/package-lock.json +++ b/packages/artifact/package-lock.json @@ -22,8 +22,7 @@ "crypto": "^1.0.1", "jwt-decode": "^3.1.2", "twirp-ts": "^2.5.0", - "unzip-stream": "^0.3.1", - "util": "^0.12.5" + "unzip-stream": "^0.3.1" }, "devDependencies": { "@types/archiver": "^5.3.2", @@ -561,17 +560,6 @@ "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==" }, - "node_modules/available-typed-arrays": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/available-typed-arrays/-/available-typed-arrays-1.0.5.tgz", - "integrity": "sha512-DMD0KiN46eipeziST1LPP/STfDU0sufISXmjSgvVsoU2tqxctQeASejWcfNtxYKqETM1UxQ8sp2OrSBWpHY6sw==", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -676,19 +664,6 @@ "node": ">=0.2.0" } }, - "node_modules/call-bind": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/call-bind/-/call-bind-1.0.5.tgz", - "integrity": "sha512-C3nQxfFZxFRVoJoGKKI8y3MOEo129NQ+FgQ08iye+Mk4zNZZGdjfs06bVTr+DBSlA66Q2VEcMki/cUCP4SercQ==", - "dependencies": { - "function-bind": "^1.1.2", - "get-intrinsic": "^1.2.1", - "set-function-length": "^1.1.1" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/camel-case": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/camel-case/-/camel-case-4.1.2.tgz", @@ -781,19 +756,6 @@ "integrity": "sha512-VxBKmeNcqQdiUQUW2Tzq0t377b54N2bMtXO/qiLa+6eRRmmC4qT3D4OnTGoT/U6O9aklQ/jTwbOtRMTTY8G0Ig==", "deprecated": "This package is no longer supported. It's now a built-in Node module. If you've depended on crypto, you should switch to the one that's built-in." }, - "node_modules/define-data-property": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/define-data-property/-/define-data-property-1.1.1.tgz", - "integrity": "sha512-E7uGkTzkk1d0ByLeSc6ZsFS79Axg+m1P/VsgYsxHgiuc3tFSj+MjMIwe90FC4lOAZzNBdY7kkO2P2wKdsQ1vgQ==", - "dependencies": { - "get-intrinsic": "^1.2.1", - "gopd": "^1.0.1", - "has-property-descriptors": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/delayed-stream": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", @@ -835,14 +797,6 @@ "node": ">=0.8.x" } }, - "node_modules/for-each": { - "version": "0.3.3", - "resolved": "https://registry.npmjs.org/for-each/-/for-each-0.3.3.tgz", - "integrity": "sha512-jqYfLp7mo9vIyQf8ykW2v7A+2N4QjeCeI5+Dz9XraiO1ign81wjiH7Fb9vSOWvQfNtmSa4H2RoQTrrXivdUZmw==", - "dependencies": { - "is-callable": "^1.1.3" - } - }, "node_modules/form-data": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.0.tgz", @@ -866,28 +820,6 @@ "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz", "integrity": "sha512-OO0pH2lK6a0hZnAdau5ItzHPI6pUlvI7jMVnxUQRtw4owF2wk8lOSabtGDCTP4Ggrg2MbGnWO9X8K1t4+fGMDw==" }, - "node_modules/function-bind": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz", - "integrity": "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA==", - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/get-intrinsic": { - "version": "1.2.2", - "resolved": "https://registry.npmjs.org/get-intrinsic/-/get-intrinsic-1.2.2.tgz", - "integrity": "sha512-0gSo4ml/0j98Y3lngkFEot/zhiCeWsbYIlZ+uZOVgzLyLaUw7wxUL+nCTP0XJvJg1AXulJRI3UJi8GsbDuxdGA==", - "dependencies": { - "function-bind": "^1.1.2", - "has-proto": "^1.0.1", - "has-symbols": "^1.0.3", - "hasown": "^2.0.0" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/glob": { "version": "7.2.3", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.3.tgz", @@ -907,17 +839,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/gopd": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.0.1.tgz", - "integrity": "sha512-d65bNlIadxvpb/A2abVdlqKqV563juRnZ1Wtk6s1sIR8uNsXR70xqIzVqxVf1eTqDunwT2MkczEeaezCKTZhwA==", - "dependencies": { - "get-intrinsic": "^1.1.3" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/graceful-fs": { "version": "4.2.11", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", @@ -944,64 +865,6 @@ "uglify-js": "^3.1.4" } }, - "node_modules/has-property-descriptors": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/has-property-descriptors/-/has-property-descriptors-1.0.1.tgz", - "integrity": "sha512-VsX8eaIewvas0xnvinAe9bw4WfIeODpGYikiWYLH+dma0Jw6KHYqWiWfhQlgOVK8D6PvjubK5Uc4P0iIhIcNVg==", - "dependencies": { - "get-intrinsic": "^1.2.2" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/has-proto": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/has-proto/-/has-proto-1.0.1.tgz", - "integrity": "sha512-7qE+iP+O+bgF9clE5+UoBFzE65mlBiVj3tKCrlNQ0Ogwm0BjpT/gK4SlLYDMybDh5I3TCTKnPPa0oMG7JDYrhg==", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/has-symbols": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/has-symbols/-/has-symbols-1.0.3.tgz", - "integrity": "sha512-l3LCuF6MgDNwTDKkdYGEihYjt5pRPbEg46rtlmnSPlUbgmB8LOIrKJbYYFBSbnPaJexMKtiPO8hmeRjRz2Td+A==", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/has-tostringtag": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/has-tostringtag/-/has-tostringtag-1.0.0.tgz", - "integrity": "sha512-kFjcSNhnlGV1kyoGk7OXKSawH5JOb/LzUc5w9B02hOTO0dfFRjbHQKvg1d6cf3HbeUmtU9VbbV3qzZ2Teh97WQ==", - "dependencies": { - "has-symbols": "^1.0.2" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/hasown": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.0.tgz", - "integrity": "sha512-vUptKVTpIJhcczKBbgnS+RtcuYMB8+oNzPK2/Hp3hanz8JmpATdmmgLgSaadVREkDm+e2giHwY3ZRkyjSIDDFA==", - "dependencies": { - "function-bind": "^1.1.2" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/ieee754": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", @@ -1035,46 +898,6 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, - "node_modules/is-arguments": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/is-arguments/-/is-arguments-1.1.1.tgz", - "integrity": "sha512-8Q7EARjzEnKpt/PCD7e1cgUS0a6X8u5tdSiMqXhojOdoV9TsMsiO+9VLC5vAmO8N7/GmXn7yjR8qnA6bVAEzfA==", - "dependencies": { - "call-bind": "^1.0.2", - "has-tostringtag": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-callable": { - "version": "1.2.7", - "resolved": "https://registry.npmjs.org/is-callable/-/is-callable-1.2.7.tgz", - "integrity": "sha512-1BC0BVFhS/p0qtw6enp8e+8OD0UrK0oFLztSjNzhcKA3WDuJxxAPXzPuPtKkjEY9UUoEWlX/8fgKeu2S8i9JTA==", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/is-generator-function": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/is-generator-function/-/is-generator-function-1.0.10.tgz", - "integrity": "sha512-jsEjy9l3yiXEQ+PsXdmBwEPcOxaXWLspKdplFUVI9vq1iZgIekeC0L167qeu86czQaxed3q/Uzuw0swL0irL8A==", - "dependencies": { - "has-tostringtag": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/is-plain-object": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/is-plain-object/-/is-plain-object-5.0.0.tgz", @@ -1083,20 +906,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-typed-array": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/is-typed-array/-/is-typed-array-1.1.12.tgz", - "integrity": "sha512-Z14TF2JNG8Lss5/HMqt0//T9JeHXttXy5pH/DBU4vi98ozO2btxzq9MwYDZYnKwU8nRsz/+GVFVRDq3DkVuSPg==", - "dependencies": { - "which-typed-array": "^1.1.11" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/isarray": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz", @@ -1419,20 +1228,6 @@ "resolved": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==" }, - "node_modules/set-function-length": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.1.1.tgz", - "integrity": "sha512-VoaqjbBJKiWtg4yRcKBQ7g7wnGnLV3M8oLvVWwOk2PdYY6PEFegR1vezXR0tw6fZGF9csVakIRjrJiy2veSBFQ==", - "dependencies": { - "define-data-property": "^1.1.1", - "get-intrinsic": "^1.2.1", - "gopd": "^1.0.1", - "has-property-descriptors": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/shiki": { "version": "0.14.5", "resolved": "https://registry.npmjs.org/shiki/-/shiki-0.14.5.tgz", @@ -1637,18 +1432,6 @@ "mkdirp": "^0.5.1" } }, - "node_modules/util": { - "version": "0.12.5", - "resolved": "https://registry.npmjs.org/util/-/util-0.12.5.tgz", - "integrity": "sha512-kZf/K6hEIrWHI6XqOFUiiMa+79wE/D8Q+NCNAWclkyg3b4d2k7s0QGepNjiABc+aR3N1PAyHL7p6UcLY6LmrnA==", - "dependencies": { - "inherits": "^2.0.3", - "is-arguments": "^1.0.4", - "is-generator-function": "^1.0.7", - "is-typed-array": "^1.1.3", - "which-typed-array": "^1.1.2" - } - }, "node_modules/util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", @@ -1688,24 +1471,6 @@ "webidl-conversions": "^3.0.0" } }, - "node_modules/which-typed-array": { - "version": "1.1.13", - "resolved": "https://registry.npmjs.org/which-typed-array/-/which-typed-array-1.1.13.tgz", - "integrity": "sha512-P5Nra0qjSncduVPEAr7xhoF5guty49ArDTwzJ/yNuPIbZppyRxFQsRCWrocxIY+CnMVG+qfbU2FmDKyvSGClow==", - "dependencies": { - "available-typed-arrays": "^1.0.5", - "call-bind": "^1.0.4", - "for-each": "^0.3.3", - "gopd": "^1.0.1", - "has-tostringtag": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/wordwrap": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/wordwrap/-/wordwrap-1.0.0.tgz", diff --git a/packages/artifact/package.json b/packages/artifact/package.json index 6cdde611..c4f826c6 100644 --- a/packages/artifact/package.json +++ b/packages/artifact/package.json @@ -53,8 +53,7 @@ "crypto": "^1.0.1", "jwt-decode": "^3.1.2", "twirp-ts": "^2.5.0", - "unzip-stream": "^0.3.1", - "util": "^0.12.5" + "unzip-stream": "^0.3.1" }, "devDependencies": { "@types/archiver": "^5.3.2", diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 6ff72a02..0bc4a508 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -1,14 +1,13 @@ import fs from 'fs/promises' import * as github from '@actions/github' import * as core from '@actions/core' -// import * as httpClient from '@actions/http-client' -import * as util from 'util' +import * as httpClient from '@actions/http-client' import unzip from 'unzip-stream' import { DownloadArtifactOptions, DownloadArtifactResponse } from '../shared/interfaces' -// import {getUserAgentString} from '../shared/user-agent' +import {getUserAgentString} from '../shared/user-agent' import {getGitHubWorkspaceDir} from '../shared/config' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' import { @@ -18,7 +17,6 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' -import {BlobClient, BlobDownloadOptions} from '@azure/storage-blob' const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -40,21 +38,41 @@ async function exists(path: string): Promise { } async function streamExtract(url: string, directory: string): Promise { - const blobClient = new BlobClient(url) - const options: BlobDownloadOptions = { - maxRetryRequests: 5, - onProgress: (progress) => core.info(`Download bytes ${progress.loadedBytes}`) + let retryCount = 0 + while (retryCount < 3) { + try { + await streamExtractInternal(url, directory) + return + } catch (error) { + retryCount++ + core.warning(`Failed to download artifact. Retrying...`) + } } - const response = await blobClient.download(0, undefined, options) + throw new Error(`Artifact download failed after ${retryCount} retries.`) +} + +async function streamExtractInternal( + url: string, + directory: string +): Promise { + const client = new httpClient.HttpClient(getUserAgentString()) + const response = await client.get(url) + + if (response.message.statusCode !== 200) { + throw new Error( + `Unexpected HTTP response from blob storage: ${response.message.statusCode} ${response.message.statusMessage}` + ) + } return new Promise((resolve, reject) => { - response.readableStreamBody - ?.on('error', reject) - .on('aborted', reject) - .pipe(unzip.Extract({path: directory})) - .on('error', reject) - .on('close', resolve) + const zipStream = unzip.Extract({path: directory}) + try { + response.message.pipe(zipStream).on('close', resolve).on('error', reject) + } catch (error) { + zipStream.end() + reject(error) + } }) } From 34a411f3c0f1f7386969a141589b5ececd628151 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Wed, 20 Dec 2023 13:59:31 -0500 Subject: [PATCH 08/31] add timeout in between data chunks --- .../internal/download/download-artifact.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 0bc4a508..697a10c8 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -67,11 +67,30 @@ async function streamExtractInternal( return new Promise((resolve, reject) => { const zipStream = unzip.Extract({path: directory}) + + const timeout = 30 * 1000 + const timerFn = (): void => { + throw new Error(`Blob storage chunk did not respond in ${timeout}ms `) + } + let timer = setTimeout(timerFn, timeout) + try { - response.message.pipe(zipStream).on('close', resolve).on('error', reject) + response.message + .on('data', () => { + clearTimeout(timer) + timer = setTimeout(timerFn, timeout) + }) + .pipe(zipStream) + .on('close', () => { + clearTimeout(timer) + resolve() + }) + .on('error', reject) } catch (error) { zipStream.end() reject(error) + } finally { + clearTimeout(timer) } }) } From d6f3ee93b8e8f669a89200765442748bd7a08df2 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Wed, 20 Dec 2023 14:37:13 -0500 Subject: [PATCH 09/31] reject don't throw --- packages/artifact/src/internal/download/download-artifact.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 697a10c8..110375ab 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -70,7 +70,7 @@ async function streamExtractInternal( const timeout = 30 * 1000 const timerFn = (): void => { - throw new Error(`Blob storage chunk did not respond in ${timeout}ms `) + reject(new Error(`Blob storage chunk did not respond in ${timeout}ms `)) } let timer = setTimeout(timerFn, timeout) From c33724abbd6a193e7109d9c8afe087efa65d2b06 Mon Sep 17 00:00:00 2001 From: srryan Date: Wed, 20 Dec 2023 15:45:19 -0500 Subject: [PATCH 10/31] update to http client --- .../artifact/src/internal/download/download-artifact.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 110375ab..2caa92ab 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -39,13 +39,15 @@ async function exists(path: string): Promise { async function streamExtract(url: string, directory: string): Promise { let retryCount = 0 - while (retryCount < 3) { + while (retryCount < 5) { try { await streamExtractInternal(url, directory) return } catch (error) { retryCount++ - core.warning(`Failed to download artifact. Retrying...`) + core.warning(`Failed to download artifact. Retrying in 5 seconds...`) + // wait 5 seconds before retrying + await new Promise(resolve => setTimeout(resolve, 5000)) } } @@ -70,6 +72,7 @@ async function streamExtractInternal( const timeout = 30 * 1000 const timerFn = (): void => { + zipStream.end() reject(new Error(`Blob storage chunk did not respond in ${timeout}ms `)) } let timer = setTimeout(timerFn, timeout) @@ -82,6 +85,7 @@ async function streamExtractInternal( }) .pipe(zipStream) .on('close', () => { + core.debug(`zip stream: Artifact downloaded to: ${directory}`) clearTimeout(timer) resolve() }) From 03319fcffa358f46fb4dac52083e554d324f194d Mon Sep 17 00:00:00 2001 From: srryan Date: Wed, 20 Dec 2023 18:08:00 -0500 Subject: [PATCH 11/31] client fixes for retries + logging --- .../internal/download/download-artifact.ts | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 2caa92ab..101253bd 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -17,6 +17,7 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' +import { clear } from 'console' const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -41,11 +42,12 @@ async function streamExtract(url: string, directory: string): Promise { let retryCount = 0 while (retryCount < 5) { try { - await streamExtractInternal(url, directory) - return + await streamExtractInternal(url, directory) + core.info(`Artifact downloaded successfully after ${retryCount} retries.`) + return } catch (error) { retryCount++ - core.warning(`Failed to download artifact. Retrying in 5 seconds...`) + core.warning(`Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...`) // wait 5 seconds before retrying await new Promise(resolve => setTimeout(resolve, 5000)) } @@ -54,10 +56,7 @@ async function streamExtract(url: string, directory: string): Promise { throw new Error(`Artifact download failed after ${retryCount} retries.`) } -async function streamExtractInternal( - url: string, - directory: string -): Promise { +async function streamExtractInternal(url: string,directory: string): Promise { const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) @@ -67,35 +66,35 @@ async function streamExtractInternal( ) } - return new Promise((resolve, reject) => { - const zipStream = unzip.Extract({path: directory}) + const timeout = 30 * 1000 // 30 seconds - const timeout = 30 * 1000 + return new Promise((resolve, reject) => { const timerFn = (): void => { - zipStream.end() - reject(new Error(`Blob storage chunk did not respond in ${timeout}ms `)) + // close response stream + core.warning("timerFn: closing response stream") + response.message.destroy(new Error(`Blob storage chunk did not respond in ${timeout}ms`)) } let timer = setTimeout(timerFn, timeout) - try { - response.message - .on('data', () => { - clearTimeout(timer) - timer = setTimeout(timerFn, timeout) - }) - .pipe(zipStream) - .on('close', () => { - core.debug(`zip stream: Artifact downloaded to: ${directory}`) - clearTimeout(timer) - resolve() - }) - .on('error', reject) - } catch (error) { - zipStream.end() - reject(error) - } finally { - clearTimeout(timer) - } + response.message + .on('data', () => { + timer.refresh() + }) + .on('error', (error: Error) => { + core.warning(`response.message: Artifact download failed: ${error.message}`) + clearTimeout(timer) + reject(error) + }) + .pipe(unzip.Extract({path: directory})) + .on('close', () => { + core.info(`zip stream: Artifact downloaded to: ${directory}`) + clearTimeout(timer) + resolve() + }) + .on('error', (error: Error) => { + core.warning(`zip stream: Artifact download failed: ${error.message}`) + reject(error) + }) }) } @@ -193,6 +192,7 @@ export async function downloadArtifactInternal( core.info(`Starting download of artifact to: ${downloadPath}`) await streamExtract(signedUrl, downloadPath) core.info(`Artifact download completed successfully.`) + process.exit(0) } catch (error) { throw new Error(`Unable to download and extract artifact: ${error.message}`) } From ecb4df89bfb69eb5ddd27b2802e0263383c4ca9c Mon Sep 17 00:00:00 2001 From: srryan Date: Wed, 20 Dec 2023 18:23:47 -0500 Subject: [PATCH 12/31] remove the exit --- packages/artifact/src/internal/download/download-artifact.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 101253bd..b7698102 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -192,7 +192,6 @@ export async function downloadArtifactInternal( core.info(`Starting download of artifact to: ${downloadPath}`) await streamExtract(signedUrl, downloadPath) core.info(`Artifact download completed successfully.`) - process.exit(0) } catch (error) { throw new Error(`Unable to download and extract artifact: ${error.message}`) } From ff2c52461146fe3f983a215d2865cc101b410729 Mon Sep 17 00:00:00 2001 From: bethanyj28 Date: Thu, 21 Dec 2023 09:25:34 -0500 Subject: [PATCH 13/31] lint and format --- .../internal/download/download-artifact.ts | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index b7698102..38837642 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -17,7 +17,6 @@ import { } from '../../generated' import {getBackendIdsFromToken} from '../shared/util' import {ArtifactNotFoundError} from '../shared/errors' -import { clear } from 'console' const scrubQueryParameters = (url: string): string => { const parsed = new URL(url) @@ -42,12 +41,14 @@ async function streamExtract(url: string, directory: string): Promise { let retryCount = 0 while (retryCount < 5) { try { - await streamExtractInternal(url, directory) - core.info(`Artifact downloaded successfully after ${retryCount} retries.`) - return + await streamExtractInternal(url, directory) + core.info(`Artifact downloaded successfully after ${retryCount} retries.`) + return } catch (error) { retryCount++ - core.warning(`Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...`) + core.warning( + `Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...` + ) // wait 5 seconds before retrying await new Promise(resolve => setTimeout(resolve, 5000)) } @@ -56,7 +57,10 @@ async function streamExtract(url: string, directory: string): Promise { throw new Error(`Artifact download failed after ${retryCount} retries.`) } -async function streamExtractInternal(url: string,directory: string): Promise { +async function streamExtractInternal( + url: string, + directory: string +): Promise { const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) @@ -71,17 +75,21 @@ async function streamExtractInternal(url: string,directory: string): Promise { const timerFn = (): void => { // close response stream - core.warning("timerFn: closing response stream") - response.message.destroy(new Error(`Blob storage chunk did not respond in ${timeout}ms`)) + core.warning('timerFn: closing response stream') + response.message.destroy( + new Error(`Blob storage chunk did not respond in ${timeout}ms`) + ) } - let timer = setTimeout(timerFn, timeout) + const timer = setTimeout(timerFn, timeout) response.message .on('data', () => { timer.refresh() }) .on('error', (error: Error) => { - core.warning(`response.message: Artifact download failed: ${error.message}`) + core.warning( + `response.message: Artifact download failed: ${error.message}` + ) clearTimeout(timer) reject(error) }) From f482643a6ef891a1dbf4f608eff4760ccd9dfc8f Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Thu, 21 Dec 2023 15:10:01 +0000 Subject: [PATCH 14/31] updating timeout for retries --- packages/artifact/__tests__/download-artifact.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 815a760c..bc22c95a 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -290,7 +290,7 @@ describe('download-artifact', () => { expect(mockGetArtifactFailure).toHaveBeenCalledWith( fixtures.blobStorageUrl ) - }) + }, 28000) }) describe('internal', () => { From 0d399758140f3f628ccb53b4329718129bec964d Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Thu, 21 Dec 2023 18:31:01 +0000 Subject: [PATCH 15/31] updating test with blob timeouts --- .../__tests__/download-artifact.test.ts | 134 +++++++++++++++++- 1 file changed, 131 insertions(+), 3 deletions(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index bc22c95a..37c1aa5e 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -12,7 +12,7 @@ import { downloadArtifactPublic } from '../src/internal/download/download-artifact' import {getUserAgentString} from '../src/internal/shared/user-agent' -import {noopLogs} from './common' +//import {noopLogs} from './common' import * as config from '../src/internal/shared/config' import {ArtifactServiceClientJSON} from '../src/generated' import * as util from '../src/internal/shared/util' @@ -87,7 +87,7 @@ const expectExtractedArchive = async (dir: string): Promise => { } const setup = async (): Promise => { - noopLogs() + //noopLogs() await fs.promises.mkdir(testDir, {recursive: true}) await createTestArchive() @@ -120,6 +120,32 @@ const mockGetArtifactFailure = jest.fn(() => { } }) +const mockDelayBlobResponse = jest.fn(async () => { + // await new Promise(r => setTimeout(r, 31000)) + const timeout = 31000 + const message = new http.IncomingMessage(new net.Socket()) + setTimeout(() => { + message.emit('data', 'test response') + setTimeout(() => { + message.statusCode = 200 + message.push('OK') + message.push(null) + }, timeout) + }, timeout) + // message + // .on('data', async () => { + // await new Promise(r => setTimeout(r, 31000)) + // }) + // .on('done', () => { + // message.statusCode = 200 + // message.push('OK') + // message.push(null) + // }) + return { + message + } +}) + describe('download-artifact', () => { describe('public', () => { beforeEach(setup) @@ -248,7 +274,56 @@ describe('download-artifact', () => { }) }) - it('should fail if blob storage response is non-200', async () => { + it('should fail if blob storage storage chunk does not respond within 30s', async () => { + // jest.useFakeTimers() + + const downloadArtifactMock = github.getOctokit(fixtures.token).rest + .actions.downloadArtifact as MockedDownloadArtifact + downloadArtifactMock.mockResolvedValueOnce({ + headers: { + location: fixtures.blobStorageUrl + }, + status: 302, + url: '', + data: Buffer.from('') + }) + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockDelayBlobResponse + } + } + ) + + await downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) + + // jest.runAllTimers() + + expect(downloadArtifactMock).toHaveBeenCalledWith({ + owner: fixtures.repositoryOwner, + repo: fixtures.repositoryName, + artifact_id: fixtures.artifactID, + archive_format: 'zip', + request: { + redirect: 'manual' + } + }) + // expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + await expect( + downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) + ).rejects.toBeInstanceOf(Error) + }, 40000) + it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest .actions.downloadArtifact as MockedDownloadArtifact downloadArtifactMock.mockResolvedValueOnce({ @@ -290,6 +365,59 @@ describe('download-artifact', () => { expect(mockGetArtifactFailure).toHaveBeenCalledWith( fixtures.blobStorageUrl ) + expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) + }, 28000) + + it('should retry if blob storage response is non-200 and then succeed with a 200', async () => { + const downloadArtifactMock = github.getOctokit(fixtures.token).rest + .actions.downloadArtifact as MockedDownloadArtifact + downloadArtifactMock.mockResolvedValueOnce({ + headers: { + location: fixtures.blobStorageUrl + }, + status: 302, + url: '', + data: Buffer.from('') + }) + + const mockGetArtifact = jest + .fn(mockGetArtifactSuccess) + .mockImplementationOnce(mockGetArtifactFailure) + + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + () => { + return { + get: mockGetArtifact + } + } + ) + + const response = await downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) + + expect(downloadArtifactMock).toHaveBeenCalledWith({ + owner: fixtures.repositoryOwner, + repo: fixtures.repositoryName, + artifact_id: fixtures.artifactID, + archive_format: 'zip', + request: { + redirect: 'manual' + } + }) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactFailure).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + expect(mockGetArtifactFailure).toHaveBeenCalledTimes(1) + expect(mockGetArtifactSuccess).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + expect(mockGetArtifactSuccess).toHaveBeenCalledTimes(1) + expect(response.downloadPath).toBe(fixtures.workspaceDir) }, 28000) }) From 98e1a813dbc18a10ff203e344744cc71362c9303 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Thu, 21 Dec 2023 20:22:20 +0000 Subject: [PATCH 16/31] testing ci --- package.json | 2 +- .../__tests__/download-artifact.test.ts | 24 +++++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 73a7093c..01566f9e 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "lint": "eslint packages/**/*.ts", "lint-fix": "eslint packages/**/*.ts --fix", "new-package": "scripts/create-package", - "test": "jest --testTimeout 10000" + "test": "jest --testTimeout 20000" }, "devDependencies": { "@types/jest": "^29.5.4", diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 37c1aa5e..bd59dfd2 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -295,13 +295,14 @@ describe('download-artifact', () => { } ) - await downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) - + await expect( + downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) + ).rejects.toBeInstanceOf(Error) // jest.runAllTimers() expect(downloadArtifactMock).toHaveBeenCalledWith({ @@ -313,15 +314,6 @@ describe('download-artifact', () => { redirect: 'manual' } }) - // expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) - await expect( - downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) - ).rejects.toBeInstanceOf(Error) }, 40000) it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest From 7f47ffaee2cf75b384649968529078a0a4d1d4ee Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Fri, 22 Dec 2023 03:51:47 +0000 Subject: [PATCH 17/31] committing v1 --- .../__tests__/download-artifact.test.ts | 84 +++++++++++++++---- .../internal/download/download-artifact.ts | 4 + 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index bd59dfd2..f2a74f93 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -120,27 +120,42 @@ const mockGetArtifactFailure = jest.fn(() => { } }) -const mockDelayBlobResponse = jest.fn(async () => { - // await new Promise(r => setTimeout(r, 31000)) - const timeout = 31000 +const mockDelayWritableResponse = jest.fn(async () => { + //const message = new stream.PassThrough() const message = new http.IncomingMessage(new net.Socket()) + message.statusCode = 200 + message.push('Internal Server Error') + message.push(null) + setTimeout(() => { - message.emit('data', 'test response') + message.emit('error', () => {}) + }, 31000) + return { + message + } +}) +const mockDelayBlobResponse = jest.fn(async () => { + const message = new http.IncomingMessage(new net.Socket()) + message.on('data', () => { setTimeout(() => { message.statusCode = 200 message.push('OK') message.push(null) - }, timeout) - }, timeout) - // message - // .on('data', async () => { - // await new Promise(r => setTimeout(r, 31000)) - // }) - // .on('done', () => { + }, 31000) + }) + // setTimeout(() => { + // setTimeout(() => { // message.statusCode = 200 // message.push('OK') // message.push(null) - // }) + // }, 31000) + // message.emit('data') + // }, 31000) + // message.on('done', () => { + // message.statusCode = 200 + // message.push('OK') + // message.push(null) + // }) return { message } @@ -290,12 +305,18 @@ describe('download-artifact', () => { const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( () => { return { - get: mockDelayBlobResponse + get: mockDelayWritableResponse } } ) + await downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) - await expect( + expect( downloadArtifactPublic( fixtures.artifactID, fixtures.repositoryOwner, @@ -303,7 +324,6 @@ describe('download-artifact', () => { fixtures.token ) ).rejects.toBeInstanceOf(Error) - // jest.runAllTimers() expect(downloadArtifactMock).toHaveBeenCalledWith({ owner: fixtures.repositoryOwner, @@ -314,6 +334,40 @@ describe('download-artifact', () => { redirect: 'manual' } }) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactFailure).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) + // const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( + // () => { + // return { + // get: mockDelayBlobResponse + // } + // } + // ) + + // expect(downloadArtifactMock).toHaveBeenCalledWith({ + // owner: fixtures.repositoryOwner, + // repo: fixtures.repositoryName, + // artifact_id: fixtures.artifactID, + // archive_format: 'zip', + // request: { + // redirect: 'manual' + // } + // }) + // expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + // expect(mockGetArtifactFailure).toHaveBeenCalledWith( + // fixtures.blobStorageUrl + // ) + // await expect( + // downloadArtifactPublic( + // fixtures.artifactID, + // fixtures.repositoryOwner, + // fixtures.repositoryName, + // fixtures.token + // ) + // ).rejects.toBeInstanceOf(Error) }, 40000) it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 38837642..a840ba85 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -38,6 +38,7 @@ async function exists(path: string): Promise { } async function streamExtract(url: string, directory: string): Promise { + core.info(`Stream extract started`) let retryCount = 0 while (retryCount < 5) { try { @@ -61,8 +62,11 @@ async function streamExtractInternal( url: string, directory: string ): Promise { + core.info(`Stream extract internal started`) + const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) + core.info(`Stream extract internal get called`) if (response.message.statusCode !== 200) { throw new Error( From 9d70b8a9fb0439f8ba921a7a8df09877428cd084 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Mon, 8 Jan 2024 15:20:05 +0000 Subject: [PATCH 18/31] testing reject after timeout --- .../__tests__/download-artifact.test.ts | 79 +++++++------------ 1 file changed, 28 insertions(+), 51 deletions(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index f2a74f93..7a48029b 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -309,65 +309,42 @@ describe('download-artifact', () => { } } ) - await downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) - expect( - downloadArtifactPublic( + try { + await downloadArtifactPublic( fixtures.artifactID, fixtures.repositoryOwner, fixtures.repositoryName, fixtures.token ) - ).rejects.toBeInstanceOf(Error) - expect(downloadArtifactMock).toHaveBeenCalledWith({ - owner: fixtures.repositoryOwner, - repo: fixtures.repositoryName, - artifact_id: fixtures.artifactID, - archive_format: 'zip', - request: { - redirect: 'manual' - } - }) - expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) - expect(mockGetArtifactFailure).toHaveBeenCalledWith( - fixtures.blobStorageUrl - ) - expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) - // const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( - // () => { - // return { - // get: mockDelayBlobResponse - // } - // } - // ) + expect( + downloadArtifactPublic( + fixtures.artifactID, + fixtures.repositoryOwner, + fixtures.repositoryName, + fixtures.token + ) + ).rejects.toBeInstanceOf(Error) - // expect(downloadArtifactMock).toHaveBeenCalledWith({ - // owner: fixtures.repositoryOwner, - // repo: fixtures.repositoryName, - // artifact_id: fixtures.artifactID, - // archive_format: 'zip', - // request: { - // redirect: 'manual' - // } - // }) - // expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) - // expect(mockGetArtifactFailure).toHaveBeenCalledWith( - // fixtures.blobStorageUrl - // ) - // await expect( - // downloadArtifactPublic( - // fixtures.artifactID, - // fixtures.repositoryOwner, - // fixtures.repositoryName, - // fixtures.token - // ) - // ).rejects.toBeInstanceOf(Error) + expect(downloadArtifactMock).toHaveBeenCalledWith({ + owner: fixtures.repositoryOwner, + repo: fixtures.repositoryName, + artifact_id: fixtures.artifactID, + archive_format: 'zip', + request: { + redirect: 'manual' + } + }) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + expect(mockGetArtifactFailure).toHaveBeenCalledWith( + fixtures.blobStorageUrl + ) + expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) + } catch (err) { + // eslint-disable-next-line no-console + console.log(err) + } }, 40000) it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest From 67d2d582dcaae5549aab57d5bb7a5c6c128d4300 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 16:44:12 +0000 Subject: [PATCH 19/31] adding delayed response to message body http mock --- .../__tests__/download-artifact.test.ts | 115 +++++------------- .../internal/download/download-artifact.ts | 4 +- 2 files changed, 30 insertions(+), 89 deletions(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 7a48029b..970d1fdd 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -9,7 +9,8 @@ import archiver from 'archiver' import { downloadArtifactInternal, - downloadArtifactPublic + downloadArtifactPublic, + streamExtractExternal } from '../src/internal/download/download-artifact' import {getUserAgentString} from '../src/internal/shared/user-agent' //import {noopLogs} from './common' @@ -120,47 +121,6 @@ const mockGetArtifactFailure = jest.fn(() => { } }) -const mockDelayWritableResponse = jest.fn(async () => { - //const message = new stream.PassThrough() - const message = new http.IncomingMessage(new net.Socket()) - message.statusCode = 200 - message.push('Internal Server Error') - message.push(null) - - setTimeout(() => { - message.emit('error', () => {}) - }, 31000) - return { - message - } -}) -const mockDelayBlobResponse = jest.fn(async () => { - const message = new http.IncomingMessage(new net.Socket()) - message.on('data', () => { - setTimeout(() => { - message.statusCode = 200 - message.push('OK') - message.push(null) - }, 31000) - }) - // setTimeout(() => { - // setTimeout(() => { - // message.statusCode = 200 - // message.push('OK') - // message.push(null) - // }, 31000) - // message.emit('data') - // }, 31000) - // message.on('done', () => { - // message.statusCode = 200 - // message.push('OK') - // message.push(null) - // }) - return { - message - } -}) - describe('download-artifact', () => { describe('public', () => { beforeEach(setup) @@ -290,62 +250,43 @@ describe('download-artifact', () => { }) it('should fail if blob storage storage chunk does not respond within 30s', async () => { - // jest.useFakeTimers() + // mock http client to delay response data by 30s + // + const msg = new http.IncomingMessage(new net.Socket()) + msg.statusCode = 200 - const downloadArtifactMock = github.getOctokit(fixtures.token).rest - .actions.downloadArtifact as MockedDownloadArtifact - downloadArtifactMock.mockResolvedValueOnce({ - headers: { - location: fixtures.blobStorageUrl - }, - status: 302, - url: '', - data: Buffer.from('') + const mockGet = jest.fn(async () => { + return new Promise((resolve, reject) => { + // Resolve with a 200 status code immediately + resolve({ + message: msg, + readBody: async () => { + return Promise.resolve(`{"ok": true}`) + } + }) + + // Reject with an error after 31 seconds + setTimeout(() => { + reject(new Error('Request timeout')) + }, 31000) // Timeout after 31 seconds + }) }) + const mockHttpClient = (HttpClient as jest.Mock).mockImplementation( () => { return { - get: mockDelayWritableResponse + get: mockGet } } ) - try { - await downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) + await expect( + streamExtractExternal(fixtures.blobStorageUrl, fixtures.workspaceDir) + ).rejects.toBeInstanceOf(Error) - expect( - downloadArtifactPublic( - fixtures.artifactID, - fixtures.repositoryOwner, - fixtures.repositoryName, - fixtures.token - ) - ).rejects.toBeInstanceOf(Error) + expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) + }, 35000) - expect(downloadArtifactMock).toHaveBeenCalledWith({ - owner: fixtures.repositoryOwner, - repo: fixtures.repositoryName, - artifact_id: fixtures.artifactID, - archive_format: 'zip', - request: { - redirect: 'manual' - } - }) - expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) - expect(mockGetArtifactFailure).toHaveBeenCalledWith( - fixtures.blobStorageUrl - ) - expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) - } catch (err) { - // eslint-disable-next-line no-console - console.log(err) - } - }, 40000) it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest .actions.downloadArtifact as MockedDownloadArtifact diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index a840ba85..42dfa1cf 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -42,7 +42,7 @@ async function streamExtract(url: string, directory: string): Promise { let retryCount = 0 while (retryCount < 5) { try { - await streamExtractInternal(url, directory) + await streamExtractExternal(url, directory) core.info(`Artifact downloaded successfully after ${retryCount} retries.`) return } catch (error) { @@ -58,7 +58,7 @@ async function streamExtract(url: string, directory: string): Promise { throw new Error(`Artifact download failed after ${retryCount} retries.`) } -async function streamExtractInternal( +export async function streamExtractExternal( url: string, directory: string ): Promise { From d63a8c4d3f0f6321c6dd165d539c25eb3f379f87 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 17:13:35 +0000 Subject: [PATCH 20/31] updating package-json --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index e72dedbd..1d43fbf3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5781,9 +5781,9 @@ } }, "node_modules/follow-redirects": { - "version": "1.15.2", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.2.tgz", - "integrity": "sha512-VQLG33o04KaQ8uYi2tVNbdrWp1QWxNNea+nmIB4EVM28v0hmP17z7aG1+wAkNzVq4KeXTq3221ye5qTJP91JwA==", + "version": "1.15.4", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.4.tgz", + "integrity": "sha512-Cr4D/5wlrb0z9dgERpUL3LrmPKVDsETIJhaCMeDfuFYcqa5bldGV6wBsAN6X/vxlXQtFBMrXdXxdL8CbDTGniw==", "dev": true, "funding": [ { From e19b629130ce52d2d14204d020e0e7c9d6ae6d48 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 18:45:26 +0000 Subject: [PATCH 21/31] increasing timeout --- packages/artifact/__tests__/download-artifact.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index 970d1fdd..cd1896aa 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -330,7 +330,7 @@ describe('download-artifact', () => { fixtures.blobStorageUrl ) expect(mockGetArtifactFailure).toHaveBeenCalledTimes(5) - }, 28000) + }, 38000) it('should retry if blob storage response is non-200 and then succeed with a 200', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest From 58ec2bdcc9b33da137f7c621395c2fb482e143cd Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 18:55:50 +0000 Subject: [PATCH 22/31] increase timeout --- jest.config.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jest.config.js b/jest.config.js index 101666d6..da496052 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,5 +7,6 @@ module.exports = { transform: { '^.+\\.ts$': 'ts-jest' }, - verbose: true -} \ No newline at end of file + verbose: true, + testTimeout: 70000 +} From 8a6aae0a1634c48dbc80571c55549095604e9d4e Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:03:41 +0000 Subject: [PATCH 23/31] updating global timeout --- jest.config.js | 3 +-- package.json | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index da496052..105aae23 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,6 +7,5 @@ module.exports = { transform: { '^.+\\.ts$': 'ts-jest' }, - verbose: true, - testTimeout: 70000 + verbose: tru } diff --git a/package.json b/package.json index 01566f9e..886033c3 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "lint": "eslint packages/**/*.ts", "lint-fix": "eslint packages/**/*.ts --fix", "new-package": "scripts/create-package", - "test": "jest --testTimeout 20000" + "test": "jest --testTimeout 70000" }, "devDependencies": { "@types/jest": "^29.5.4", From 47157e5ade481cb731bda787ac1b842262da68bb Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:05:11 +0000 Subject: [PATCH 24/31] fixing true --- jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index 105aae23..f25dbdc9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -7,5 +7,5 @@ module.exports = { transform: { '^.+\\.ts$': 'ts-jest' }, - verbose: tru + verbose: true } From d617670abc8cc06c5f6fd82c9f927d6198ded0d4 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:23:57 +0000 Subject: [PATCH 25/31] updating timer; removing logs --- package.json | 2 +- packages/artifact/__tests__/download-artifact.test.ts | 7 +++---- .../artifact/src/internal/download/download-artifact.ts | 6 +----- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 886033c3..ea3ac2ed 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "lint": "eslint packages/**/*.ts", "lint-fix": "eslint packages/**/*.ts --fix", "new-package": "scripts/create-package", - "test": "jest --testTimeout 70000" + "test": "jest --testTimeout 60000" }, "devDependencies": { "@types/jest": "^29.5.4", diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts index cd1896aa..1c0c9b0d 100644 --- a/packages/artifact/__tests__/download-artifact.test.ts +++ b/packages/artifact/__tests__/download-artifact.test.ts @@ -13,7 +13,7 @@ import { streamExtractExternal } from '../src/internal/download/download-artifact' import {getUserAgentString} from '../src/internal/shared/user-agent' -//import {noopLogs} from './common' +import {noopLogs} from './common' import * as config from '../src/internal/shared/config' import {ArtifactServiceClientJSON} from '../src/generated' import * as util from '../src/internal/shared/util' @@ -88,7 +88,7 @@ const expectExtractedArchive = async (dir: string): Promise => { } const setup = async (): Promise => { - //noopLogs() + noopLogs() await fs.promises.mkdir(testDir, {recursive: true}) await createTestArchive() @@ -251,7 +251,6 @@ describe('download-artifact', () => { it('should fail if blob storage storage chunk does not respond within 30s', async () => { // mock http client to delay response data by 30s - // const msg = new http.IncomingMessage(new net.Socket()) msg.statusCode = 200 @@ -285,7 +284,7 @@ describe('download-artifact', () => { ).rejects.toBeInstanceOf(Error) expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString()) - }, 35000) + }, 35000) // add longer timeout to allow for timer to run out it('should fail if blob storage response is non-200 after 5 retries', async () => { const downloadArtifactMock = github.getOctokit(fixtures.token).rest diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 42dfa1cf..21a1bdb1 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -38,7 +38,6 @@ async function exists(path: string): Promise { } async function streamExtract(url: string, directory: string): Promise { - core.info(`Stream extract started`) let retryCount = 0 while (retryCount < 5) { try { @@ -66,7 +65,6 @@ export async function streamExtractExternal( const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) - core.info(`Stream extract internal get called`) if (response.message.statusCode !== 200) { throw new Error( @@ -78,8 +76,6 @@ export async function streamExtractExternal( return new Promise((resolve, reject) => { const timerFn = (): void => { - // close response stream - core.warning('timerFn: closing response stream') response.message.destroy( new Error(`Blob storage chunk did not respond in ${timeout}ms`) ) @@ -91,7 +87,7 @@ export async function streamExtractExternal( timer.refresh() }) .on('error', (error: Error) => { - core.warning( + core.debug( `response.message: Artifact download failed: ${error.message}` ) clearTimeout(timer) From 2124ef24137ba4c153729cd1f323af25b4212f08 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:36:26 +0000 Subject: [PATCH 26/31] cleaning up logs --- .../artifact/src/internal/download/download-artifact.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 21a1bdb1..7201b2f2 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -42,11 +42,10 @@ async function streamExtract(url: string, directory: string): Promise { while (retryCount < 5) { try { await streamExtractExternal(url, directory) - core.info(`Artifact downloaded successfully after ${retryCount} retries.`) return } catch (error) { retryCount++ - core.warning( + core.debug( `Failed to download artifact after ${retryCount} retries due to ${error.message}. Retrying in 5 seconds...` ) // wait 5 seconds before retrying @@ -61,11 +60,8 @@ export async function streamExtractExternal( url: string, directory: string ): Promise { - core.info(`Stream extract internal started`) - const client = new httpClient.HttpClient(getUserAgentString()) const response = await client.get(url) - if (response.message.statusCode !== 200) { throw new Error( `Unexpected HTTP response from blob storage: ${response.message.statusCode} ${response.message.statusMessage}` From 7549d1b2187f0ca0ac71072272698bc1cc9ec463 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:42:04 +0000 Subject: [PATCH 27/31] removing info logs --- packages/artifact/src/internal/download/download-artifact.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts index 7201b2f2..dc54f6fe 100644 --- a/packages/artifact/src/internal/download/download-artifact.ts +++ b/packages/artifact/src/internal/download/download-artifact.ts @@ -91,12 +91,10 @@ export async function streamExtractExternal( }) .pipe(unzip.Extract({path: directory})) .on('close', () => { - core.info(`zip stream: Artifact downloaded to: ${directory}`) clearTimeout(timer) resolve() }) .on('error', (error: Error) => { - core.warning(`zip stream: Artifact download failed: ${error.message}`) reject(error) }) }) From f37c445bc5ef316917b91580601771fe1a50ed91 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:46:17 +0000 Subject: [PATCH 29/31] reverting jest --- jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index f25dbdc9..101666d6 100644 --- a/jest.config.js +++ b/jest.config.js @@ -8,4 +8,4 @@ module.exports = { '^.+\\.ts$': 'ts-jest' }, verbose: true -} +} \ No newline at end of file From c1ded1dc4deef7c895cad774e8dc14f4cbcc1e30 Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:47:02 +0000 Subject: [PATCH 30/31] appeasing linter --- jest.config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index 101666d6..15077ce9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -8,4 +8,5 @@ module.exports = { '^.+\\.ts$': 'ts-jest' }, verbose: true -} \ No newline at end of file +} +// adding extra line for linter From 439cd9b37e3c3a245101d6c87d5e91326f6d3c4f Mon Sep 17 00:00:00 2001 From: Vallie Joseph Date: Tue, 9 Jan 2024 19:47:25 +0000 Subject: [PATCH 31/31] appeasing linter --- jest.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index 15077ce9..f25dbdc9 100644 --- a/jest.config.js +++ b/jest.config.js @@ -9,4 +9,3 @@ module.exports = { }, verbose: true } -// adding extra line for linter