From d82237c1d195af4f0a2b8fd8245908036bdbab1e Mon Sep 17 00:00:00 2001 From: Josh Pinkney Date: Thu, 16 Jan 2025 10:45:39 -0500 Subject: [PATCH 1/2] refactor(core): Make HttpResourceFetcher platform agnostic --- .../src/lambda/commands/downloadLambda.ts | 2 +- .../resourcefetcher/httpResourceFetcher.ts | 100 +++----------- .../node/httpResourceFetcher.ts | 129 ++++++++++++++++++ .../core/src/shared/utilities/cliUtils.ts | 2 +- 4 files changed, 149 insertions(+), 84 deletions(-) create mode 100644 packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts diff --git a/packages/core/src/lambda/commands/downloadLambda.ts b/packages/core/src/lambda/commands/downloadLambda.ts index 16128ce5701..815ff2576e9 100644 --- a/packages/core/src/lambda/commands/downloadLambda.ts +++ b/packages/core/src/lambda/commands/downloadLambda.ts @@ -14,7 +14,7 @@ import { LaunchConfiguration, getReferencedHandlerPaths } from '../../shared/deb import { makeTemporaryToolkitFolder, fileExists, tryRemoveFolder } from '../../shared/filesystemUtilities' import * as localizedText from '../../shared/localizedText' import { getLogger } from '../../shared/logger' -import { HttpResourceFetcher } from '../../shared/resourcefetcher/httpResourceFetcher' +import { HttpResourceFetcher } from '../../shared/resourcefetcher/node/httpResourceFetcher' import { createCodeAwsSamDebugConfig } from '../../shared/sam/debugger/awsSamDebugConfiguration' import * as pathutils from '../../shared/utilities/pathUtils' import { localize } from '../../shared/utilities/vsCodeUtils' diff --git a/packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts b/packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts index f2da4ba98aa..20e33138fba 100644 --- a/packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts +++ b/packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts @@ -3,38 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'fs' // eslint-disable-line no-restricted-imports -import * as http from 'http' -import * as https from 'https' -import * as stream from 'stream' -import got, { Response, RequestError, CancelError } from 'got' -import urlToOptions from 'got/dist/source/core/utils/url-to-options' -import Request from 'got/dist/source/core' import { VSCODE_EXTENSION_ID } from '../extensions' import { getLogger, Logger } from '../logger' import { ResourceFetcher } from './resourcefetcher' -import { Timeout, CancellationError, CancelEvent } from '../utilities/timeoutUtils' -import { isCloud9 } from '../extensionUtilities' -import { Headers } from 'got/dist/source/core' - -// XXX: patched Got module for compatability with older VS Code versions (e.g. Cloud9) -// `got` has also deprecated `urlToOptions` -const patchedGot = got.extend({ - request: (url, options, callback) => { - if (url.protocol === 'https:') { - return https.request({ ...options, ...urlToOptions(url) }, callback) - } - return http.request({ ...options, ...urlToOptions(url) }, callback) - }, -}) - -/** Promise that resolves/rejects when all streams close. Can also access streams directly. */ -type FetcherResult = Promise & { - /** Download stream piped to `fsStream`. */ - requestStream: Request // `got` doesn't add the correct types to 'on' for some reason - /** Stream writing to the file system. */ - fsStream: fs.WriteStream -} +import { Timeout, CancelEvent } from '../utilities/timeoutUtils' +import request, { RequestError } from '../request' type RequestHeaders = { eTag?: string; gZip?: boolean } @@ -65,20 +38,8 @@ export class HttpResourceFetcher implements ResourceFetcher { * * @param pipeLocation Optionally pipe the download to a file system location */ - public get(): Promise - public get(pipeLocation: string): FetcherResult - public get(pipeLocation?: string): Promise | FetcherResult { + public get(): Promise { this.logger.verbose(`downloading: ${this.logText()}`) - - if (pipeLocation) { - const result = this.pipeGetRequest(pipeLocation, this.params.timeout) - result.fsStream.on('exit', () => { - this.logger.verbose(`downloaded: ${this.logText()}`) - }) - - return result - } - return this.downloadRequest() } @@ -94,15 +55,15 @@ export class HttpResourceFetcher implements ResourceFetcher { public async getNewETagContent(eTag?: string): Promise<{ content?: string; eTag: string }> { const response = await this.getResponseFromGetRequest(this.params.timeout, { eTag, gZip: true }) - const eTagResponse = response.headers.etag + const eTagResponse = response.headers.get('etag') if (!eTagResponse) { throw new Error(`This URL does not support E-Tags. Cannot use this function for: ${this.url.toString()}`) } // NOTE: Even with use of `gzip` encoding header, the response content is uncompressed. // Most likely due to the http request library uncompressing it for us. - let contents: string | undefined = response.body.toString() - if (response.statusCode === 304) { + let contents: string | undefined = await response.text() + if (response.status === 304) { // Explanation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match contents = undefined this.logger.verbose(`E-Tag, ${eTagResponse}, matched. No content downloaded from: ${this.url}`) @@ -119,7 +80,8 @@ export class HttpResourceFetcher implements ResourceFetcher { private async downloadRequest(): Promise { try { // HACK(?): receiving JSON as a string without `toString` makes it so we can't deserialize later - const contents = (await this.getResponseFromGetRequest(this.params.timeout)).body.toString() + const resp = await this.getResponseFromGetRequest(this.params.timeout) + const contents = (await resp.text()).toString() if (this.params.onSuccess) { this.params.onSuccess(contents) } @@ -128,10 +90,10 @@ export class HttpResourceFetcher implements ResourceFetcher { return contents } catch (err) { - const error = err as CancelError | RequestError + const error = err as RequestError this.logger.verbose( `Error downloading ${this.logText()}: %s`, - error.message ?? error.code ?? error.response?.statusMessage ?? error.response?.statusCode + error.message ?? error.code ?? error.response.statusText ?? error.response.status ) return undefined } @@ -145,56 +107,30 @@ export class HttpResourceFetcher implements ResourceFetcher { getLogger().debug(`Download for "${this.logText()}" ${event.agent === 'user' ? 'cancelled' : 'timed out'}`) } - // TODO: make pipeLocation a vscode.Uri - private pipeGetRequest(pipeLocation: string, timeout?: Timeout): FetcherResult { - const requester = isCloud9() ? patchedGot : got - const requestStream = requester.stream(this.url, { headers: this.buildRequestHeaders() }) - const fsStream = fs.createWriteStream(pipeLocation) - - const done = new Promise((resolve, reject) => { - const pipe = stream.pipeline(requestStream, fsStream, (err) => { - if (err instanceof RequestError) { - return reject(Object.assign(new Error('Failed to download file'), { code: err.code })) - } - err ? reject(err) : resolve() - }) - - const cancelListener = timeout?.token.onCancellationRequested((event) => { - this.logCancellation(event) - pipe.destroy(new CancellationError(event.agent)) - }) - - pipe.on('close', () => cancelListener?.dispose()) - }) - - return Object.assign(done, { requestStream, fsStream }) - } - - private async getResponseFromGetRequest(timeout?: Timeout, headers?: RequestHeaders): Promise> { - const requester = isCloud9() ? patchedGot : got - const promise = requester(this.url, { + private async getResponseFromGetRequest(timeout?: Timeout, headers?: RequestHeaders): Promise { + const req = request.fetch('GET', this.url, { headers: this.buildRequestHeaders(headers), }) const cancelListener = timeout?.token.onCancellationRequested((event) => { this.logCancellation(event) - promise.cancel(new CancellationError(event.agent).message) + req.cancel() }) - return promise.finally(() => cancelListener?.dispose()) + return req.response.finally(() => cancelListener?.dispose()) } private buildRequestHeaders(requestHeaders?: RequestHeaders): Headers { - const headers: Headers = {} + const headers = new Headers() - headers['User-Agent'] = VSCODE_EXTENSION_ID.awstoolkit + headers.set('User-Agent', VSCODE_EXTENSION_ID.awstoolkit) if (requestHeaders?.eTag !== undefined) { - headers['If-None-Match'] = requestHeaders.eTag + headers.set('If-None-Match', requestHeaders.eTag) } if (requestHeaders?.gZip) { - headers['Accept-Encoding'] = 'gzip' + headers.set('Accept-Encoding', 'gzip') } return headers diff --git a/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts b/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts new file mode 100644 index 00000000000..37c46e9e234 --- /dev/null +++ b/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts @@ -0,0 +1,129 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'fs' // eslint-disable-line no-restricted-imports +import * as http from 'http' +import * as https from 'https' +import * as stream from 'stream' +import got, { RequestError } from 'got' +import urlToOptions from 'got/dist/source/core/utils/url-to-options' +import Request from 'got/dist/source/core' +import { VSCODE_EXTENSION_ID } from '../../extensions' +import { getLogger, Logger } from '../../logger' +import { Timeout, CancellationError, CancelEvent } from '../../utilities/timeoutUtils' +import { isCloud9 } from '../../extensionUtilities' +import { Headers } from 'got/dist/source/core' + +// XXX: patched Got module for compatability with older VS Code versions (e.g. Cloud9) +// `got` has also deprecated `urlToOptions` +const patchedGot = got.extend({ + request: (url, options, callback) => { + if (url.protocol === 'https:') { + return https.request({ ...options, ...urlToOptions(url) }, callback) + } + return http.request({ ...options, ...urlToOptions(url) }, callback) + }, +}) + +/** Promise that resolves/rejects when all streams close. Can also access streams directly. */ +type FetcherResult = Promise & { + /** Download stream piped to `fsStream`. */ + requestStream: Request // `got` doesn't add the correct types to 'on' for some reason + /** Stream writing to the file system. */ + fsStream: fs.WriteStream +} + +type RequestHeaders = { eTag?: string; gZip?: boolean } + +/** + * Legacy HTTP Resource Fetcher used specifically for streaming information. + * If you do not need streaming or want multi platform support use + */ +export class HttpResourceFetcher { + private readonly logger: Logger = getLogger() + + /** + * + * @param url URL to fetch a response body from via the `get` call + * @param params Additional params for the fetcher + * @param {boolean} params.showUrl Whether or not to the URL in log statements. + * @param {string} params.friendlyName If URL is not shown, replaces the URL with this text. + * @param {function} params.onSuccess Function to execute on successful request. No effect if piping to a location. + * @param {Timeout} params.timeout Timeout token to abort/cancel the request. Similar to `AbortSignal`. + */ + public constructor( + private readonly url: string, + private readonly params: { + showUrl: boolean + friendlyName?: string + timeout?: Timeout + } + ) {} + + /** + * Returns the contents of the resource, or undefined if the resource could not be retrieved. + * + * @param pipeLocation Optionally pipe the download to a file system location + */ + public get(pipeLocation: string): FetcherResult { + this.logger.verbose(`downloading: ${this.logText()}`) + + const result = this.pipeGetRequest(pipeLocation, this.params.timeout) + result.fsStream.on('exit', () => { + this.logger.verbose(`downloaded: ${this.logText()}`) + }) + + return result + } + + private logText(): string { + return this.params.showUrl ? this.url : (this.params.friendlyName ?? 'resource from URL') + } + + private logCancellation(event: CancelEvent) { + getLogger().debug(`Download for "${this.logText()}" ${event.agent === 'user' ? 'cancelled' : 'timed out'}`) + } + + // TODO: make pipeLocation a vscode.Uri + private pipeGetRequest(pipeLocation: string, timeout?: Timeout): FetcherResult { + const requester = isCloud9() ? patchedGot : got + const requestStream = requester.stream(this.url, { headers: this.buildRequestHeaders() }) + const fsStream = fs.createWriteStream(pipeLocation) + + const done = new Promise((resolve, reject) => { + const pipe = stream.pipeline(requestStream, fsStream, (err) => { + if (err instanceof RequestError) { + return reject(Object.assign(new Error('Failed to download file'), { code: err.code })) + } + err ? reject(err) : resolve() + }) + + const cancelListener = timeout?.token.onCancellationRequested((event) => { + this.logCancellation(event) + pipe.destroy(new CancellationError(event.agent)) + }) + + pipe.on('close', () => cancelListener?.dispose()) + }) + + return Object.assign(done, { requestStream, fsStream }) + } + + private buildRequestHeaders(requestHeaders?: RequestHeaders): Headers { + const headers: Headers = {} + + headers['User-Agent'] = VSCODE_EXTENSION_ID.awstoolkit + + if (requestHeaders?.eTag !== undefined) { + headers['If-None-Match'] = requestHeaders.eTag + } + + if (requestHeaders?.gZip) { + headers['Accept-Encoding'] = 'gzip' + } + + return headers + } +} diff --git a/packages/core/src/shared/utilities/cliUtils.ts b/packages/core/src/shared/utilities/cliUtils.ts index a247ed19489..b52ae10b023 100644 --- a/packages/core/src/shared/utilities/cliUtils.ts +++ b/packages/core/src/shared/utilities/cliUtils.ts @@ -11,7 +11,7 @@ import * as vscode from 'vscode' import { getIdeProperties } from '../extensionUtilities' import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../filesystemUtilities' import { getLogger } from '../logger' -import { HttpResourceFetcher } from '../resourcefetcher/httpResourceFetcher' +import { HttpResourceFetcher } from '../resourcefetcher/node/httpResourceFetcher' import { ChildProcess } from './processUtils' import * as nls from 'vscode-nls' From 86155f9bea3d8fac3734c924c6c2a383d28e492b Mon Sep 17 00:00:00 2001 From: Josh Pinkney Date: Fri, 17 Jan 2025 10:28:03 -0500 Subject: [PATCH 2/2] fix --- .../core/src/shared/resourcefetcher/node/httpResourceFetcher.ts | 2 +- .../core/src/test/awsService/appBuilder/walkthrough.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts b/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts index 37c46e9e234..d801e8c5027 100644 --- a/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts +++ b/packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts @@ -39,7 +39,7 @@ type RequestHeaders = { eTag?: string; gZip?: boolean } /** * Legacy HTTP Resource Fetcher used specifically for streaming information. - * If you do not need streaming or want multi platform support use + * Only kept around until web streams are compatible with node streams */ export class HttpResourceFetcher { private readonly logger: Logger = getLogger() diff --git a/packages/core/src/test/awsService/appBuilder/walkthrough.test.ts b/packages/core/src/test/awsService/appBuilder/walkthrough.test.ts index 72dfb5c0ae2..5528bc0a496 100644 --- a/packages/core/src/test/awsService/appBuilder/walkthrough.test.ts +++ b/packages/core/src/test/awsService/appBuilder/walkthrough.test.ts @@ -23,7 +23,7 @@ import { getTestWindow } from '../../shared/vscode/window' import { AwsClis, installCli } from '../../../shared/utilities/cliUtils' import { ChildProcess } from '../../../shared/utilities/processUtils' import { assertTelemetryCurried } from '../../testUtil' -import { HttpResourceFetcher } from '../../../shared/resourcefetcher/httpResourceFetcher' +import { HttpResourceFetcher } from '../../../shared/resourcefetcher/node/httpResourceFetcher' import { SamCliInfoInvocation } from '../../../shared/sam/cli/samCliInfo' import { CodeScansState } from '../../../codewhisperer'