From 01f74aa87885b1dbc0fdd7a318e35f4a075deb5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 17 Nov 2021 10:39:28 +0100 Subject: [PATCH] Add `AbortSignal` support to `UrlReader` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/proud-bottles-cheat.md | 5 ++ packages/backend-common/api-report.md | 10 ++-- packages/backend-common/package.json | 1 + .../src/reading/AwsS3UrlReader.ts | 29 +++++++----- .../src/reading/AzureUrlReader.ts | 47 +++++++++++-------- .../src/reading/BitbucketUrlReader.ts | 30 +++++++----- .../src/reading/FetchUrlReader.ts | 1 + .../src/reading/GithubUrlReader.ts | 5 +- .../src/reading/GitlabUrlReader.ts | 18 +++++-- packages/backend-common/src/reading/types.ts | 28 +++++++++++ 10 files changed, 121 insertions(+), 53 deletions(-) create mode 100644 .changeset/proud-bottles-cheat.md diff --git a/.changeset/proud-bottles-cheat.md b/.changeset/proud-bottles-cheat.md new file mode 100644 index 0000000000..0c724352c7 --- /dev/null +++ b/.changeset/proud-bottles-cheat.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Add `AbortSignal` support to `UrlReader` diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 7242c32a84..e3119e78a8 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -6,6 +6,7 @@ /// /// +import { AbortSignal as AbortSignal_2 } from 'node-abort-controller'; import { AwsS3Integration } from '@backstage/integration'; import { AzureIntegration } from '@backstage/integration'; import { BitbucketIntegration } from '@backstage/integration'; @@ -48,7 +49,7 @@ export class AwsS3UrlReader implements UrlReader { // (undocumented) read(url: string): Promise; // (undocumented) - readTree(url: string): Promise; + readTree(url: string, options?: ReadTreeOptions): Promise; // (undocumented) readUrl(url: string, options?: ReadUrlOptions): Promise; // (undocumented) @@ -72,7 +73,7 @@ export class AzureUrlReader implements UrlReader { // (undocumented) readTree(url: string, options?: ReadTreeOptions): Promise; // (undocumented) - readUrl(url: string, _options?: ReadUrlOptions): Promise; + readUrl(url: string, options?: ReadUrlOptions): Promise; // (undocumented) search(url: string, options?: SearchOptions): Promise; // (undocumented) @@ -94,7 +95,7 @@ export class BitbucketUrlReader implements UrlReader { // (undocumented) readTree(url: string, options?: ReadTreeOptions): Promise; // (undocumented) - readUrl(url: string, _options?: ReadUrlOptions): Promise; + readUrl(url: string, options?: ReadUrlOptions): Promise; // (undocumented) search(url: string, options?: SearchOptions): Promise; // (undocumented) @@ -418,6 +419,7 @@ export type ReadTreeOptions = { }, ): boolean; etag?: string; + signal?: AbortSignal_2; }; // @public @@ -471,6 +473,7 @@ export type ReadTreeResponseFile = { // @public export type ReadUrlOptions = { etag?: string; + signal?: AbortSignal_2; }; // @public @@ -508,6 +511,7 @@ export type RunContainerOptions = { // @public export type SearchOptions = { etag?: string; + signal?: AbortSignal_2; }; // @public diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 03ae1fbe62..16ce62c7d7 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -62,6 +62,7 @@ "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", + "node-abort-controller": "^3.0.1", "raw-body": "^2.4.1", "selfsigned": "^1.10.7", "stoppable": "^1.1.0", diff --git a/packages/backend-common/src/reading/AwsS3UrlReader.ts b/packages/backend-common/src/reading/AwsS3UrlReader.ts index 05a14e26ec..0d806fcfaa 100644 --- a/packages/backend-common/src/reading/AwsS3UrlReader.ts +++ b/packages/backend-common/src/reading/AwsS3UrlReader.ts @@ -18,6 +18,7 @@ import aws, { Credentials, S3 } from 'aws-sdk'; import { CredentialsOptions } from 'aws-sdk/lib/credentials'; import { ReaderFactory, + ReadTreeOptions, ReadTreeResponse, ReadTreeResponseFactory, ReadUrlOptions, @@ -92,7 +93,7 @@ export class AwsS3UrlReader implements UrlReader { ) {} /** - * If accesKeyId and secretAccessKey are missing, the standard credentials provider chain will be used: + * If accessKeyId and secretAccessKey are missing, the standard credentials provider chain will be used: * https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.html */ private static buildCredentials( @@ -154,9 +155,10 @@ export class AwsS3UrlReader implements UrlReader { }; } - const response = this.deps.s3.getObject(params); - const buffer = await getRawBody(response.createReadStream()); - const etag = (await response.promise()).ETag; + const request = this.deps.s3.getObject(params); + options?.signal?.addEventListener('abort', () => request.abort()); + const buffer = await getRawBody(request.createReadStream()); + const etag = (await request.promise()).ETag; return { buffer: async () => buffer, @@ -171,7 +173,10 @@ export class AwsS3UrlReader implements UrlReader { } } - async readTree(url: string): Promise { + async readTree( + url: string, + options?: ReadTreeOptions, + ): Promise { try { const { path, bucket, region } = parseURL(url); const allObjects: ObjectList = []; @@ -180,13 +185,13 @@ export class AwsS3UrlReader implements UrlReader { let output: ListObjectsV2Output; do { aws.config.update({ region: region }); - output = await this.deps.s3 - .listObjectsV2({ - Bucket: bucket, - ContinuationToken: continuationToken, - Prefix: path, - }) - .promise(); + const request = this.deps.s3.listObjectsV2({ + Bucket: bucket, + ContinuationToken: continuationToken, + Prefix: path, + }); + options?.signal?.addEventListener('abort', () => request.abort()); + output = await request.promise(); if (output.Contents) { output.Contents.forEach(contents => { allObjects.push(contents); diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index 6015b044bc..82d1192b17 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -55,21 +55,34 @@ export class AzureUrlReader implements UrlReader { ) {} async read(url: string): Promise { + const response = await this.readUrl(url); + return response.buffer(); + } + + async readUrl( + url: string, + options?: ReadUrlOptions, + ): Promise { + // TODO: etag is not implemented yet. + const { signal } = options ?? {}; + const builtUrl = getAzureFileFetchUrl(url); let response: Response; try { - response = await fetch( - builtUrl, - getAzureRequestOptions(this.integration.config), - ); + response = await fetch(builtUrl, { + ...getAzureRequestOptions(this.integration.config), + ...(signal && { signal }), + }); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); } // for private repos when PAT is not valid, Azure API returns a http status code 203 with sign in page html if (response.ok && response.status !== 203) { - return Buffer.from(await response.arrayBuffer()); + return { + buffer: async () => Buffer.from(await response.arrayBuffer()), + }; } const message = `${url} could not be read as ${builtUrl}, ${response.status} ${response.statusText}`; @@ -79,19 +92,12 @@ export class AzureUrlReader implements UrlReader { throw new Error(message); } - async readUrl( - url: string, - _options?: ReadUrlOptions, - ): Promise { - // TODO etag is not implemented yet. - const buffer = await this.read(url); - return { buffer: async () => buffer }; - } - async readTree( url: string, options?: ReadTreeOptions, ): Promise { + const { etag, filter, signal } = options ?? {}; + // TODO: Support filepath based reading tree feature like other providers // Get latest commit SHA @@ -109,16 +115,16 @@ export class AzureUrlReader implements UrlReader { } const commitSha = (await commitsAzureResponse.json()).value[0].commitId; - if (options?.etag && options.etag === commitSha) { + if (etag && etag === commitSha) { throw new NotModifiedError(); } - const archiveAzureResponse = await fetch( - getAzureDownloadUrl(url), - getAzureRequestOptions(this.integration.config, { + const archiveAzureResponse = await fetch(getAzureDownloadUrl(url), { + ...getAzureRequestOptions(this.integration.config, { Accept: 'application/zip', }), - ); + ...(signal && { signal }), + }); if (!archiveAzureResponse.ok) { const message = `Failed to read tree from ${url}, ${archiveAzureResponse.status} ${archiveAzureResponse.statusText}`; if (archiveAzureResponse.status === 404) { @@ -139,7 +145,7 @@ export class AzureUrlReader implements UrlReader { return await this.deps.treeResponseFactory.fromZipArchive({ stream: archiveAzureResponse.body as unknown as Readable, etag: commitSha, - filter: options?.filter, + filter, subpath, }); } @@ -158,6 +164,7 @@ export class AzureUrlReader implements UrlReader { const tree = await this.readTree(treeUrl.toString(), { etag: options?.etag, + signal: options?.signal, filter: p => (matcher ? matcher.match(p) : true), }); const files = await tree.files(); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 4ae0ea37a8..c1a113a50d 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -77,18 +77,33 @@ export class BitbucketUrlReader implements UrlReader { } async read(url: string): Promise { + const response = await this.readUrl(url); + return response.buffer(); + } + + async readUrl( + url: string, + options?: ReadUrlOptions, + ): Promise { + // TODO: etag is not supported yet + const { signal } = options ?? {}; const bitbucketUrl = getBitbucketFileFetchUrl(url, this.integration.config); - const options = getBitbucketRequestOptions(this.integration.config); + const requestOptions = getBitbucketRequestOptions(this.integration.config); let response: Response; try { - response = await fetch(bitbucketUrl.toString(), options); + response = await fetch(bitbucketUrl.toString(), { + ...requestOptions, + ...(signal && { signal }), + }); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); } if (response.ok) { - return Buffer.from(await response.arrayBuffer()); + return { + buffer: async () => Buffer.from(await response.arrayBuffer()), + }; } const message = `${url} could not be read as ${bitbucketUrl}, ${response.status} ${response.statusText}`; @@ -98,15 +113,6 @@ export class BitbucketUrlReader implements UrlReader { throw new Error(message); } - async readUrl( - url: string, - _options?: ReadUrlOptions, - ): Promise { - // TODO etag is not implemented yet. - const buffer = await this.read(url); - return { buffer: async () => buffer }; - } - async readTree( url: string, options?: ReadTreeOptions, diff --git a/packages/backend-common/src/reading/FetchUrlReader.ts b/packages/backend-common/src/reading/FetchUrlReader.ts index 732d3b9f59..8fc378d515 100644 --- a/packages/backend-common/src/reading/FetchUrlReader.ts +++ b/packages/backend-common/src/reading/FetchUrlReader.ts @@ -73,6 +73,7 @@ export class FetchUrlReader implements UrlReader { headers: { ...(options?.etag && { 'If-None-Match': options.etag }), }, + signal: options?.signal, }); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index ca16e23fec..755e0f17e6 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -110,6 +110,7 @@ export class GithubUrlReader implements UrlReader { ...(options?.etag && { 'If-None-Match': options.etag }), Accept: 'application/vnd.github.v3.raw', }, + signal: options?.signal, }); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); @@ -164,7 +165,7 @@ export class GithubUrlReader implements UrlReader { repoDetails.repo.archive_url, commitSha, filepath, - { headers }, + { headers, signal: options?.signal }, options, ); } @@ -188,7 +189,7 @@ export class GithubUrlReader implements UrlReader { repoDetails.repo.archive_url, commitSha, filepath, - { headers }, + { headers, signal: options?.signal }, ); return { files, etag: commitSha }; diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 3fc9673e8b..128aed3c85 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -66,6 +66,7 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadUrlOptions, ): Promise { + const { etag, signal } = options ?? {}; const builtUrl = await getGitLabFileFetchUrl(url, this.integration.config); let response: Response; @@ -73,8 +74,9 @@ export class GitlabUrlReader implements UrlReader { response = await fetch(builtUrl, { headers: { ...getGitLabRequestOptions(this.integration.config).headers, - ...(options?.etag && { 'If-None-Match': options.etag }), + ...(etag && { 'If-None-Match': etag }), }, + ...(signal && { signal }), }); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); @@ -102,6 +104,7 @@ export class GitlabUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { + const { etag, signal } = options ?? {}; const { ref, full_name, filepath } = parseGitUrl(url); // Use GitLab API to get the default branch @@ -140,7 +143,10 @@ export class GitlabUrlReader implements UrlReader { full_name, )}/repository/commits?${commitsReqParams.toString()}`, ).toString(), - getGitLabRequestOptions(this.integration.config), + { + ...getGitLabRequestOptions(this.integration.config), + ...(signal && { signal }), + }, ); if (!commitsGitlabResponse.ok) { const message = `Failed to read tree (branch) from ${url}, ${commitsGitlabResponse.status} ${commitsGitlabResponse.statusText}`; @@ -152,7 +158,7 @@ export class GitlabUrlReader implements UrlReader { const commitSha = (await commitsGitlabResponse.json())[0].id; - if (options?.etag && options.etag === commitSha) { + if (etag && etag === commitSha) { throw new NotModifiedError(); } @@ -161,7 +167,10 @@ export class GitlabUrlReader implements UrlReader { `${this.integration.config.apiBaseUrl}/projects/${encodeURIComponent( full_name, )}/repository/archive?sha=${branch}`, - getGitLabRequestOptions(this.integration.config), + { + ...getGitLabRequestOptions(this.integration.config), + ...(signal && { signal }), + }, ); if (!archiveGitLabResponse.ok) { const message = `Failed to read tree (archive) from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; @@ -191,6 +200,7 @@ export class GitlabUrlReader implements UrlReader { const tree = await this.readTree(treeUrl, { etag: options?.etag, + signal: options?.signal, filter: path => matcher.match(stripFirstDirectoryFromPath(path)), }); const files = await tree.files(); diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index 7e0904f18b..16edf81ba6 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -17,6 +17,7 @@ import { Readable } from 'stream'; import { Logger } from 'winston'; import { Config } from '@backstage/config'; +import { AbortSignal } from 'node-abort-controller'; /** * A generic interface for fetching plain data from URLs. @@ -101,6 +102,15 @@ export type ReadUrlOptions = { * of the response along with a new ETag. */ etag?: string; + + /** + * An abort signal to pass down to the underlying request. + * + * @remarks + * + * Not all reader implementations may take this field into account. + */ + signal?: AbortSignal; }; /** @@ -165,6 +175,15 @@ export type ReadTreeOptions = { * rest of the response along with a new ETag. */ etag?: string; + + /** + * An abort signal to pass down to the underlying request. + * + * @remarks + * + * Not all reader implementations may take this field into account. + */ + signal?: AbortSignal; }; /** @@ -291,6 +310,15 @@ export type SearchOptions = { * search will return the rest of SearchResponse along with a new etag. */ etag?: string; + + /** + * An abort signal to pass down to the underlying request. + * + * @remarks + * + * Not all reader implementations may take this field into account. + */ + signal?: AbortSignal; }; /**