Skip to content

Commit

Permalink
refactor(core): Make HttpResourceFetcher platform agnostic (aws#6379)
Browse files Browse the repository at this point in the history
## Problem
- Resource fetcher currently only works on node. This causes some
implements to use got directly, some use fetch directly, some use
httpResourceFetcher depending on their use case

## Solution
- Make HttpResourceFetcher platform independent so it can run on node +
web
- Move the streaming based HttpResourceFetcher to a node specific folder

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
  • Loading branch information
jpinkney-aws authored and karanA-aws committed Jan 17, 2025
1 parent 8c883c9 commit 5211728
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 85 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/lambda/commands/downloadLambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
100 changes: 18 additions & 82 deletions packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> & {
/** 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 }

Expand Down Expand Up @@ -65,20 +38,8 @@ export class HttpResourceFetcher implements ResourceFetcher {
*
* @param pipeLocation Optionally pipe the download to a file system location
*/
public get(): Promise<string | undefined>
public get(pipeLocation: string): FetcherResult
public get(pipeLocation?: string): Promise<string | undefined> | FetcherResult {
public get(): Promise<string | undefined> {
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()
}

Expand All @@ -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}`)
Expand All @@ -119,7 +80,8 @@ export class HttpResourceFetcher implements ResourceFetcher {
private async downloadRequest(): Promise<string | undefined> {
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)
}
Expand All @@ -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
}
Expand All @@ -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<void>((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<Response<string>> {
const requester = isCloud9() ? patchedGot : got
const promise = requester(this.url, {
private async getResponseFromGetRequest(timeout?: Timeout, headers?: RequestHeaders): Promise<Response> {
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
Expand Down
129 changes: 129 additions & 0 deletions packages/core/src/shared/resourcefetcher/node/httpResourceFetcher.ts
Original file line number Diff line number Diff line change
@@ -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<void> & {
/** 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.
* Only kept around until web streams are compatible with node streams
*/
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<void>((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
}
}
2 changes: 1 addition & 1 deletion packages/core/src/shared/utilities/cliUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down

0 comments on commit 5211728

Please sign in to comment.