From e7179b57bdba192acfdb439c03099e6629e98f6a Mon Sep 17 00:00:00 2001 From: Mike Maietta Date: Thu, 28 Jul 2022 13:28:41 -0700 Subject: [PATCH] feat: Adding timeout to publisher config for api requests and uploads (#7028) * Adding timeout to publish config. Adding test for validating too short of a timeout. Expect upload to throw/reject Request timed out. Adding null type in docs and updating documentation to state 2min default --- .changeset/chatty-trains-agree.md | 8 +++ docs/api/electron-builder.md | 1 + docs/configuration/publish.md | 15 +++++ package.json | 1 + packages/app-builder-lib/scheme.json | 64 +++++++++++++++++++ .../src/electron/electronVersion.ts | 4 +- .../src/publish/BitbucketPublisher.ts | 2 + .../src/publish/KeygenPublisher.ts | 5 ++ .../src/publish/PublishManager.ts | 4 ++ .../builder-util-runtime/src/httpExecutor.ts | 16 ++--- .../src/publishOptions.ts | 7 ++ .../electron-publish/src/gitHubPublisher.ts | 2 + packages/electron-publish/src/publisher.ts | 23 ++++++- pnpm-lock.yaml | 8 +-- test/src/ArtifactPublisherTest.ts | 15 ++++- 15 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 .changeset/chatty-trains-agree.md diff --git a/.changeset/chatty-trains-agree.md b/.changeset/chatty-trains-agree.md new file mode 100644 index 0000000000..051bce9917 --- /dev/null +++ b/.changeset/chatty-trains-agree.md @@ -0,0 +1,8 @@ +--- +"app-builder-lib": minor +"builder-util-runtime": minor +"builder-util": minor +"electron-publish": minor +--- + +feat: Adding timeout to publisher config for api requests and uploads diff --git a/docs/api/electron-builder.md b/docs/api/electron-builder.md index 9f7660a8e1..2d133713a3 100644 --- a/docs/api/electron-builder.md +++ b/docs/api/electron-builder.md @@ -1304,6 +1304,7 @@ return path.join(target.outDir, __${target.name}-${getArtifactArchName(arc
  • fileContent module:global.Buffer | “undefined”
  • arch Arch | “undefined”
  • safeArtifactName String | “undefined”
    • +
  • timeout Number | “undefined”

    • HttpPublisher ⇐ Publisher

    diff --git a/docs/configuration/publish.md b/docs/configuration/publish.md index 3c2b8535a7..da1be2dd36 100644 --- a/docs/configuration/publish.md +++ b/docs/configuration/publish.md @@ -132,6 +132,9 @@ In all publish options File Macros are

  • requestHeaders module:http.OutgoingHttpHeaders - Any custom request headers

    • +
  • +

    timeout = 60000 Number | “undefined” - Request timeout in milliseconds. (Default is 2 minutes; O is ignored)

    +

    • GithubOptions

    GitHub options.

    @@ -179,6 +182,9 @@ Define GH_TOKEN environment variable.

  • requestHeaders module:http.OutgoingHttpHeaders - Any custom request headers

    • +
  • +

    timeout = 60000 Number | “undefined” - Request timeout in milliseconds. (Default is 2 minutes; O is ignored)

    +

    • SnapStoreOptions

    Snap Store options.

    @@ -196,6 +202,9 @@ Define GH_TOKEN environment variable.

  • requestHeaders module:http.OutgoingHttpHeaders - Any custom request headers

    • +
  • +

    timeout = 60000 Number | “undefined” - Request timeout in milliseconds. (Default is 2 minutes; O is ignored)

    +

    • SpacesOptions

    DigitalOcean Spaces options. @@ -228,6 +237,9 @@ Define KEYGEN_TOKEN environment variable.

  • requestHeaders module:http.OutgoingHttpHeaders - Any custom request headers

    • +
  • +

    timeout = 60000 Number | “undefined” - Request timeout in milliseconds. (Default is 2 minutes; O is ignored)

    +

    • BitbucketOptions

    Bitbucket options. @@ -256,6 +268,9 @@ Define BITBUCKET_TOKEN environment variable.

  • requestHeaders module:http.OutgoingHttpHeaders - Any custom request headers

    • +
  • +

    timeout = 60000 Number | “undefined” - Request timeout in milliseconds. (Default is 2 minutes; O is ignored)

    +

    • S3Options

    Amazon S3 options. diff --git a/package.json b/package.json index a3cb481067..52a29498de 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "generate-changeset": "pnpm changeset", "generate-docs": "pnpm compile && pnpm jsdoc && pnpm jsdoc2md2html", "generate-schema": "typescript-json-schema packages/app-builder-lib/tsconfig-scheme.json Configuration --out packages/app-builder-lib/scheme.json --noExtraProps --useTypeOfKeyword --strictNullChecks --required && node ./scripts/fix-schema.js", + "generate-all": "pnpm generate-schema && pnpm generate-docs && pnpm prettier", "ci:test": "node ./test/out/helpers/runTests.js", "ci:version": "pnpm changelog && changeset version && node scripts/update-package-version-export.js && pnpm run generate-schema && pnpm run generate-docs && pnpm run prettier && git add .", "ci:publish": "pnpm compile && pnpm publish -r && changeset tag", diff --git a/packages/app-builder-lib/scheme.json b/packages/app-builder-lib/scheme.json index dee2e547e7..dbc5d32e20 100644 --- a/packages/app-builder-lib/scheme.json +++ b/packages/app-builder-lib/scheme.json @@ -381,6 +381,14 @@ "description": "Repository slug/name", "type": "string" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "token": { "description": "The access token to support auto-update from private bitbucket repositories.", "type": [ @@ -478,6 +486,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updateProvider": { "description": "The Provider to provide UpdateInfo regarding available updates. Required\nto use custom providers with electron-updater.", "typeof": "function" @@ -1358,6 +1374,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updaterCacheDirName": { "type": [ "null", @@ -1482,6 +1506,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "token": { "description": "The access token to support auto-update from private github repositories. Never specify it in the configuration files. Only for [setFeedURL](/auto-update#appupdatersetfeedurloptions).", "type": [ @@ -1573,6 +1605,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updaterCacheDirName": { "type": [ "null", @@ -4876,6 +4916,14 @@ "default": "STANDARD", "description": "The type of storage to use for the object." }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updaterCacheDirName": { "type": [ "null", @@ -5324,6 +5372,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updaterCacheDirName": { "type": [ "null", @@ -5409,6 +5465,14 @@ "$ref": "#/definitions/OutgoingHttpHeaders", "description": "Any custom request headers" }, + "timeout": { + "default": 60000, + "description": "Request timeout in milliseconds. (Default is 2 minutes; O is ignored)", + "type": [ + "null", + "number" + ] + }, "updaterCacheDirName": { "type": [ "null", diff --git a/packages/app-builder-lib/src/electron/electronVersion.ts b/packages/app-builder-lib/src/electron/electronVersion.ts index 2de29afe1b..6caebd76e7 100644 --- a/packages/app-builder-lib/src/electron/electronVersion.ts +++ b/packages/app-builder-lib/src/electron/electronVersion.ts @@ -31,7 +31,7 @@ export async function getElectronVersionFromInstalled(projectDir: string) { for (const name of electronPackages) { try { return (await readJson(path.join(projectDir, "node_modules", name, "package.json"))).version - } catch (e) { + } catch (e: any) { if (e.code !== "ENOENT") { log.warn({ name, error: e }, `cannot read electron version package.json`) } @@ -44,7 +44,7 @@ export async function getElectronPackage(projectDir: string) { for (const name of electronPackages) { try { return await readJson(path.join(projectDir, "node_modules", name, "package.json")) - } catch (e) { + } catch (e: any) { if (e.code !== "ENOENT") { log.warn({ name, error: e }, `cannot find electron in package.json`) } diff --git a/packages/app-builder-lib/src/publish/BitbucketPublisher.ts b/packages/app-builder-lib/src/publish/BitbucketPublisher.ts index 63de458e2f..58a89d264c 100644 --- a/packages/app-builder-lib/src/publish/BitbucketPublisher.ts +++ b/packages/app-builder-lib/src/publish/BitbucketPublisher.ts @@ -49,6 +49,7 @@ export class BitbucketPublisher extends HttpPublisher { hostname: this.hostname, path: this.basePath, headers: form.getHeaders(), + timeout: this.info.timeout || undefined, } await httpExecutor.doApiRequest(configureRequestOptions(upload, this.auth, "POST"), this.context.cancellationToken, it => form.pipe(it)) return fileName @@ -59,6 +60,7 @@ export class BitbucketPublisher extends HttpPublisher { const req: RequestOptions = { hostname: this.hostname, path: `${this.basePath}/${filename}`, + timeout: this.info.timeout || undefined, } await httpExecutor.request(configureRequestOptions(req, this.auth, "DELETE"), this.context.cancellationToken) } diff --git a/packages/app-builder-lib/src/publish/KeygenPublisher.ts b/packages/app-builder-lib/src/publish/KeygenPublisher.ts index 6061e775a7..383f888234 100644 --- a/packages/app-builder-lib/src/publish/KeygenPublisher.ts +++ b/packages/app-builder-lib/src/publish/KeygenPublisher.ts @@ -139,6 +139,7 @@ export class KeygenPublisher extends HttpPublisher { headers: { "Content-Length": dataLength, }, + timeout: this.info.timeout || undefined, } await httpExecutor.doApiRequest(configureRequestOptions(upload, null, "PUT"), this.context.cancellationToken, requestProcessor) @@ -154,6 +155,7 @@ export class KeygenPublisher extends HttpPublisher { "Keygen-Version": "1.1", Prefer: "no-redirect", }, + timeout: this.info.timeout || undefined, } const data: RecursivePartial = { @@ -211,6 +213,7 @@ export class KeygenPublisher extends HttpPublisher { Accept: "application/vnd.api+json", "Keygen-Version": "1.1", }, + timeout: this.info.timeout || undefined, } return parseJson(httpExecutor.request(configureRequestOptions(req, this.auth, "GET"), this.context.cancellationToken, null)) @@ -225,6 +228,7 @@ export class KeygenPublisher extends HttpPublisher { Accept: "application/vnd.api+json", "Keygen-Version": "1.1", }, + timeout: this.info.timeout || undefined, } const data: RecursivePartial = { @@ -257,6 +261,7 @@ export class KeygenPublisher extends HttpPublisher { Accept: "application/vnd.api+json", "Keygen-Version": "1.1", }, + timeout: this.info.timeout || undefined, } await httpExecutor.request(configureRequestOptions(req, this.auth, "DELETE"), this.context.cancellationToken) } diff --git a/packages/app-builder-lib/src/publish/PublishManager.ts b/packages/app-builder-lib/src/publish/PublishManager.ts index 20a59a7b54..fd3f5becb9 100644 --- a/packages/app-builder-lib/src/publish/PublishManager.ts +++ b/packages/app-builder-lib/src/publish/PublishManager.ts @@ -167,6 +167,10 @@ export class PublishManager implements PublishContext { return } + if (publishConfig.timeout) { + event.timeout = publishConfig.timeout + } + this.taskManager.addTask(publisher.upload(event)) } diff --git a/packages/builder-util-runtime/src/httpExecutor.ts b/packages/builder-util-runtime/src/httpExecutor.ts index 7a304649e7..0bca9b0c37 100644 --- a/packages/builder-util-runtime/src/httpExecutor.ts +++ b/packages/builder-util-runtime/src/httpExecutor.ts @@ -111,11 +111,11 @@ export abstract class HttpExecutor { const request = this.createRequest(options, (response: any) => { try { this.handleResponse(response, options, cancellationToken, resolve, reject, redirectCount, requestProcessor) - } catch (e) { + } catch (e: any) { reject(e) } }) - this.addErrorAndTimeoutHandlers(request, reject) + this.addErrorAndTimeoutHandlers(request, reject, options.timeout) this.addRedirectHandlers(request, options, reject, redirectCount, options => { this.doApiRequest(options, cancellationToken, requestProcessor, redirectCount).then(resolve).catch(reject) }) @@ -130,8 +130,8 @@ export abstract class HttpExecutor { // not required for NodeJS } - addErrorAndTimeoutHandlers(request: any, reject: (error: Error) => void) { - this.addTimeOutHandler(request, reject) + addErrorAndTimeoutHandlers(request: any, reject: (error: Error) => void, timeout = 60 * 1000) { + this.addTimeOutHandler(request, reject, timeout) request.on("error", reject) request.on("aborted", () => { reject(new Error("Request has been aborted by the server")) @@ -213,7 +213,7 @@ Please double check that your authentication token is correct. Due to security r } // noinspection JSUnusedLocalSymbols - abstract createRequest(options: any, callback: (response: any) => void): T + abstract createRequest(options: RequestOptions, callback: (response: any) => void): T async downloadToBuffer(url: URL, options: DownloadOptions): Promise { return await options.cancellationToken.createPromise((resolve, reject, onCancel) => { @@ -313,7 +313,7 @@ Please double check that your authentication token is correct. Due to security r options.responseHandler(response, options.callback) } }) - this.addErrorAndTimeoutHandlers(request, options.callback) + this.addErrorAndTimeoutHandlers(request, options.callback, requestOptions.timeout) this.addRedirectHandlers(request, requestOptions, options.callback, redirectCount, requestOptions => { this.doDownload(requestOptions, options, redirectCount++) }) @@ -324,9 +324,9 @@ Please double check that your authentication token is correct. Due to security r return new Error(`Too many redirects (> ${this.maxRedirects})`) } - private addTimeOutHandler(request: any, callback: (error: Error) => void) { + private addTimeOutHandler(request: any, callback: (error: Error) => void, timeout: number) { request.on("socket", (socket: Socket) => { - socket.setTimeout(60 * 1000, () => { + socket.setTimeout(timeout, () => { request.abort() callback(new Error("Request timed out")) }) diff --git a/packages/builder-util-runtime/src/publishOptions.ts b/packages/builder-util-runtime/src/publishOptions.ts index 82cf63b5d6..363ce8eafe 100644 --- a/packages/builder-util-runtime/src/publishOptions.ts +++ b/packages/builder-util-runtime/src/publishOptions.ts @@ -46,6 +46,13 @@ export interface PublishConfiguration { * Any custom request headers */ readonly requestHeaders?: OutgoingHttpHeaders + + /** + * Request timeout in milliseconds. (Default is 2 minutes; O is ignored) + * + * @default 60000 + */ + readonly timeout?: number | null } // https://github.com/electron-userland/electron-builder/issues/3261 diff --git a/packages/electron-publish/src/gitHubPublisher.ts b/packages/electron-publish/src/gitHubPublisher.ts index d306815abd..0d7e41938f 100644 --- a/packages/electron-publish/src/gitHubPublisher.ts +++ b/packages/electron-publish/src/gitHubPublisher.ts @@ -191,6 +191,7 @@ export class GitHubPublisher extends HttpPublisher { "Content-Type": mime.getType(fileName) || "application/octet-stream", "Content-Length": dataLength, }, + timeout: this.info.timeout || undefined, }, this.token ), @@ -277,6 +278,7 @@ export class GitHubPublisher extends HttpPublisher { port: baseUrl.port as any, path: this.info.host != null && this.info.host !== "github.com" ? `/api/v3${path.startsWith("/") ? path : `/${path}`}` : path, headers: { accept: "application/vnd.github.v3+json" }, + timeout: this.info.timeout || undefined, }, token, method diff --git a/packages/electron-publish/src/publisher.ts b/packages/electron-publish/src/publisher.ts index 83dc3480bd..42e61bb745 100644 --- a/packages/electron-publish/src/publisher.ts +++ b/packages/electron-publish/src/publisher.ts @@ -32,6 +32,7 @@ export interface UploadTask { arch: Arch | null safeArtifactName?: string | null + timeout?: number | null } export abstract class Publisher { @@ -77,7 +78,21 @@ export abstract class HttpPublisher extends Publisher { const fileName = (this.useSafeArtifactName ? task.safeArtifactName : null) || basename(task.file) if (task.fileContent != null) { - await this.doUpload(fileName, task.arch || Arch.x64, task.fileContent.length, it => it.end(task.fileContent), task.file) + await this.doUpload( + fileName, + task.arch || Arch.x64, + task.fileContent.length, + (request, reject) => { + if (task.timeout) { + request.setTimeout(task.timeout, () => { + request.destroy() + reject(new Error("Request timed out")) + }) + } + return request.end(task.fileContent) + }, + task.file + ) return } @@ -93,6 +108,12 @@ export abstract class HttpPublisher extends Publisher { // reset (because can be called several times (several attempts) progressBar.update(0) } + if (task.timeout) { + request.setTimeout(task.timeout, () => { + request.destroy() + reject(new Error("Request timed out")) + }) + } return this.createReadStreamAndProgressBar(task.file, fileStat, progressBar, reject).pipe(request) }, task.file diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4cdef8d8d2..a92d099224 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8495,7 +8495,7 @@ packages: engines: {node: '>=6'} hasBin: true dependencies: - minimist: 1.2.5 + minimist: 1.2.6 /json5/2.2.1: resolution: {integrity: sha512-1hqLFMSrGHRHxav9q9gNjJ5EXznIxGVO09xQRrwplcS8qs28pZ8s8hupZAmqDwZUmVZ2Qb2jnyPOWcDH8m8dlA==} @@ -9638,7 +9638,7 @@ packages: engines: {node: '>=0.10.0'} /path-key/2.0.1: - resolution: {integrity: sha1-QRyttXTFoUDTpLGRDUDYDMn0C0A=} + resolution: {integrity: sha512-fEHGKCSmUSDPv4uoj8AlD+joPlq3peND+HRYyxFz4KPw4z926S/b8rIuFs2FYJg3BwsxJf6A9/3eIdLaYC+9Dw==} engines: {node: '>=4'} dev: true @@ -10402,7 +10402,7 @@ packages: dev: true /shebang-command/1.2.0: - resolution: {integrity: sha1-RKrGW2lbAzmJaMOfNj/uXer98eo=} + resolution: {integrity: sha512-EV3L1+UQWGor21OmnvojK36mhg+TyIKDh3iFBKBohr5xeXIhNBcx8oWdgkTEEQ+BEFFYdLRuqMfd5L84N1V5Vg==} engines: {node: '>=0.10.0'} dependencies: shebang-regex: 1.0.0 @@ -10415,7 +10415,7 @@ packages: shebang-regex: 3.0.0 /shebang-regex/1.0.0: - resolution: {integrity: sha1-2kL0l0DAtC2yypcoVxyxkMmO/qM=} + resolution: {integrity: sha512-wpoSFAxys6b2a2wHZ1XpDSgD7N9iVjg29Ph9uV/uaP9Ex/KXlkTZTeddxDPSYQpgvzKLGJke2UU0AzoGCjNIvQ==} engines: {node: '>=0.10.0'} dev: true diff --git a/test/src/ArtifactPublisherTest.ts b/test/src/ArtifactPublisherTest.ts index 50dd94f58b..1b73316032 100644 --- a/test/src/ArtifactPublisherTest.ts +++ b/test/src/ArtifactPublisherTest.ts @@ -149,11 +149,24 @@ test.ifEnv(process.env.KEYGEN_TOKEN)("Keygen upload", async () => { }) test.ifEnv(process.env.BITBUCKET_TOKEN)("Bitbucket upload", async () => { + const timeout = 0 const publisher = new BitbucketPublisher(publishContext, { provider: "bitbucket", owner: "mike-m", slug: "electron-builder-test", + timeout, } as BitbucketOptions) - const filename = await publisher.upload({ file: iconPath, arch: Arch.x64 }) + const filename = await publisher.upload({ file: iconPath, arch: Arch.x64, timeout }) await publisher.deleteRelease(filename) }) + +test.ifEnv(process.env.BITBUCKET_TOKEN)("Bitbucket upload", async () => { + const timeout = 100 + const publisher = new BitbucketPublisher(publishContext, { + provider: "bitbucket", + owner: "mike-m", + slug: "electron-builder-test", + timeout, + } as BitbucketOptions) + expect(await publisher.upload({ file: iconPath, arch: Arch.x64, timeout })).toThrowError("Request timed out") +})