diff --git a/packages/artifact/__tests__/download.test.ts b/packages/artifact/__tests__/download.test.ts index 40ab58cb75..81759d4666 100644 --- a/packages/artifact/__tests__/download.test.ts +++ b/packages/artifact/__tests__/download.test.ts @@ -12,8 +12,12 @@ import { ListArtifactsResponse, QueryArtifactResponse } from '../src/internal/contracts' +import * as stream from 'stream' +import {gzip} from 'zlib' +import {promisify} from 'util' const root = path.join(__dirname, '_temp', 'artifact-download-tests') +const defaultEncoding = 'utf8' jest.mock('../src/internal/config-variables') jest.mock('@actions/http-client') @@ -114,33 +118,49 @@ describe('Download Tests', () => { }) it('Test downloading an individual artifact with gzip', async () => { - setupDownloadItemResponse(true, 200) + const fileContents = Buffer.from( + 'gzip worked on the first try\n', + defaultEncoding + ) + const targetPath = path.join(root, 'FileA.txt') + + setupDownloadItemResponse(true, 200, false, fileContents) const downloadHttpClient = new DownloadHttpClient() const items: DownloadItem[] = [] items.push({ sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileA.txt`, - targetPath: path.join(root, 'FileA.txt') + targetPath }) await expect( downloadHttpClient.downloadSingleArtifact(items) ).resolves.not.toThrow() + + await checkDestinationFile(targetPath, fileContents) }) it('Test downloading an individual artifact without gzip', async () => { - setupDownloadItemResponse(false, 200) + const fileContents = Buffer.from( + 'plaintext worked on the first try\n', + defaultEncoding + ) + const targetPath = path.join(root, 'FileB.txt') + + setupDownloadItemResponse(false, 200, false, fileContents) const downloadHttpClient = new DownloadHttpClient() const items: DownloadItem[] = [] items.push({ sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileB.txt`, - targetPath: path.join(root, 'FileB.txt') + targetPath }) await expect( downloadHttpClient.downloadSingleArtifact(items) ).resolves.not.toThrow() + + await checkDestinationFile(targetPath, fileContents) }) it('Test retryable status codes during artifact download', async () => { @@ -148,21 +168,72 @@ describe('Download Tests', () => { // the download should successfully finish const retryableStatusCodes = [429, 502, 503, 504] for (const statusCode of retryableStatusCodes) { - setupDownloadItemResponse(false, statusCode) + const fileContents = Buffer.from('try, try again\n', defaultEncoding) + const targetPath = path.join(root, `FileC-${statusCode}.txt`) + + setupDownloadItemResponse(false, statusCode, false, fileContents) const downloadHttpClient = new DownloadHttpClient() const items: DownloadItem[] = [] items.push({ sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileC.txt`, - targetPath: path.join(root, 'FileC.txt') + targetPath }) await expect( downloadHttpClient.downloadSingleArtifact(items) ).resolves.not.toThrow() + + await checkDestinationFile(targetPath, fileContents) } }) + it('Test retry on truncated response with gzip', async () => { + const fileContents = Buffer.from( + 'Sometimes gunzip fails on the first try\n', + defaultEncoding + ) + const targetPath = path.join(root, 'FileD.txt') + + setupDownloadItemResponse(true, 200, true, fileContents) + const downloadHttpClient = new DownloadHttpClient() + + const items: DownloadItem[] = [] + items.push({ + sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileD.txt`, + targetPath + }) + + await expect( + downloadHttpClient.downloadSingleArtifact(items) + ).resolves.not.toThrow() + + await checkDestinationFile(targetPath, fileContents) + }) + + it('Test retry on truncated response without gzip', async () => { + const fileContents = Buffer.from( + 'You have to inspect the content-length header to know if you got everything\n', + defaultEncoding + ) + const targetPath = path.join(root, 'FileE.txt') + + setupDownloadItemResponse(false, 200, true, fileContents) + const downloadHttpClient = new DownloadHttpClient() + + const items: DownloadItem[] = [] + items.push({ + sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileD.txt`, + targetPath + }) + + await expect( + downloadHttpClient.downloadSingleArtifact(items) + ).resolves.not.toThrow() + + await checkDestinationFile(targetPath, fileContents) + }) + /** * Helper used to setup mocking for the HttpClient */ @@ -227,51 +298,109 @@ describe('Download Tests', () => { */ function setupDownloadItemResponse( isGzip: boolean, - firstHttpResponseCode: number + firstHttpResponseCode: number, + truncateFirstResponse: boolean, + fileContents: Buffer ): void { - jest - .spyOn(DownloadHttpClient.prototype, 'pipeResponseToFile') - .mockImplementationOnce(async () => { - return new Promise(resolve => { - resolve() - }) - }) - - jest + const spyInstance = jest .spyOn(HttpClient.prototype, 'get') .mockImplementationOnce(async () => { - const mockMessage = new http.IncomingMessage(new net.Socket()) - mockMessage.statusCode = firstHttpResponseCode - if (isGzip) { - mockMessage.headers = { - 'content-type': 'gzip' + if (firstHttpResponseCode === 200) { + const fullResponse = await constructResponse(isGzip, fileContents) + const actualResponse = truncateFirstResponse + ? fullResponse.subarray(0, 3) + : fullResponse + + return { + message: getDownloadResponseMessage( + firstHttpResponseCode, + isGzip, + fullResponse.length, + actualResponse + ), + readBody: emptyMockReadBody } - } - - return new Promise(resolve => { - resolve({ - message: mockMessage, + } else { + return { + message: getDownloadResponseMessage( + firstHttpResponseCode, + false, + 0, + null + ), readBody: emptyMockReadBody - }) - }) - }) - .mockImplementationOnce(async () => { - // chained response, if the HTTP GET function gets called again, return a successful response - const mockMessage = new http.IncomingMessage(new net.Socket()) - mockMessage.statusCode = 200 - if (isGzip) { - mockMessage.headers = { - 'content-type': 'gzip' } } + }) - return new Promise(resolve => { - resolve({ - message: mockMessage, - readBody: emptyMockReadBody - }) - }) + // set up a second mock only if we expect a retry. Otherwise this mock will affect other tests. + if (firstHttpResponseCode !== 200) { + spyInstance.mockImplementationOnce(async () => { + // chained response, if the HTTP GET function gets called again, return a successful response + const fullResponse = await constructResponse(isGzip, fileContents) + return { + message: getDownloadResponseMessage( + 200, + isGzip, + fullResponse.length, + fullResponse + ), + readBody: emptyMockReadBody + } }) + } + } + + async function constructResponse( + isGzip: boolean, + plaintext: Buffer | string + ): Promise { + if (isGzip) { + return await promisify(gzip)(plaintext) + } else if (typeof plaintext === 'string') { + return Buffer.from(plaintext, defaultEncoding) + } else { + return plaintext + } + } + + function getDownloadResponseMessage( + httpResponseCode: number, + isGzip: boolean, + contentLength: number, + response: Buffer | null + ): http.IncomingMessage { + let readCallCount = 0 + const mockMessage = new stream.Readable({ + read(size) { + switch (readCallCount++) { + case 0: + if (!!response && response.byteLength > size) { + throw new Error( + `test response larger than requested size (${size})` + ) + } + this.push(response) + break + + default: + // end the stream + this.push(null) + break + } + } + }) + + mockMessage.statusCode = httpResponseCode + mockMessage.headers = { + 'content-length': contentLength.toString() + } + + if (isGzip) { + mockMessage.headers['content-encoding'] = 'gzip' + } + + return mockMessage } /** @@ -347,4 +476,14 @@ describe('Download Tests', () => { }) }) } + + async function checkDestinationFile( + targetPath: string, + expectedContents: Buffer + ): Promise { + const fileContents = await fs.readFile(targetPath) + + expect(fileContents.byteLength).toEqual(expectedContents.byteLength) + expect(fileContents.equals(expectedContents)).toBeTruthy() + } }) diff --git a/packages/artifact/src/internal/download-http-client.ts b/packages/artifact/src/internal/download-http-client.ts index 70ea4e447a..64631c5c1f 100644 --- a/packages/artifact/src/internal/download-http-client.ts +++ b/packages/artifact/src/internal/download-http-client.ts @@ -263,26 +263,47 @@ export class DownloadHttpClient { if (isGzip) { const gunzip = zlib.createGunzip() response.message + .on('error', error => { + core.error( + `An error occurred while attempting to read the response stream` + ) + reject(error) + gunzip.close() + }) .pipe(gunzip) + .on('error', error => { + core.error( + `An error occurred while attempting to decompress the response stream` + ) + reject(error) + destinationStream.close() + }) .pipe(destinationStream) .on('close', () => { resolve() }) .on('error', error => { core.error( - `An error has been encountered while decompressing and writing a downloaded file to ${destinationStream.path}` + `An error occurred while writing a downloaded file to ${destinationStream.path}` ) reject(error) }) } else { response.message + .on('error', error => { + core.error( + `An error occurred while attempting to read the response stream` + ) + reject(error) + destinationStream.close() + }) .pipe(destinationStream) .on('close', () => { resolve() }) .on('error', error => { core.error( - `An error has been encountered while writing a downloaded file to ${destinationStream.path}` + `An error occurred while writing a downloaded file to ${destinationStream.path}` ) reject(error) })