From 261a69240dbf5b75b81a3c848b95a0c3b38fc7c6 Mon Sep 17 00:00:00 2001 From: Peter Burns Date: Fri, 1 Sep 2023 10:45:11 -0700 Subject: [PATCH 1/4] Use `using` sytax to reliably dispose of http sockets Locally this makes us compatible with node 20 --- .github/workflows/tests.yml | 7 +++++++ src/caching/github-actions-cache.ts | 31 ++++++++++++++++++++-------- src/logging/quiet/writeover-line.ts | 9 +------- src/test/util/check-script-output.ts | 1 - src/util/dispose.ts | 15 ++++++++++++++ src/util/fs.ts | 11 +--------- 6 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 src/util/dispose.ts diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ad962dc6e..07c625d59 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -28,6 +28,13 @@ jobs: os: macos-12 - node: 19 os: windows-2022 + - node: 20 + os: ubuntu-22.04 + - node: 20 + os: macos-12 + - node: 20 + os: windows-2022 + # Allow all matrix configurations to complete, instead of cancelling as # soon as one fails. Useful because we often have different kinds of diff --git a/src/caching/github-actions-cache.ts b/src/caching/github-actions-cache.ts index 0896b322a..b668031a1 100644 --- a/src/caching/github-actions-cache.ts +++ b/src/caching/github-actions-cache.ts @@ -11,6 +11,8 @@ import * as https from 'https'; import {createHash} from 'crypto'; import {scriptReferenceToString} from '../config.js'; import {getScriptDataDir} from '../util/script-data-dir.js'; +import '../util/dispose.js'; +import {fileBudget} from '../util/fs.js'; import {execFile} from 'child_process'; import type * as http from 'http'; @@ -20,7 +22,6 @@ import type {Fingerprint} from '../fingerprint.js'; import type {Logger} from '../logging/logger.js'; import type {AbsoluteEntry} from '../util/glob.js'; import type {Result} from '../error.js'; -import {fileBudget} from '../util/fs.js'; /** * Caches script output to the GitHub Actions caching service. @@ -119,7 +120,8 @@ export class GitHubActionsCache implements Cache { url.searchParams.set('keys', key); url.searchParams.set('version', version); - const {req, resPromise} = this._request(url); + using requestResult = this._request(url); + const {req, resPromise} = requestResult; req.end(); const result = await resPromise; if (!this._maybeHandleServiceDown(result, script)) { @@ -261,7 +263,8 @@ export class GitHubActionsCache implements Cache { 'content-range': `bytes ${start}-${end}/*`, }, }; - const {req, resPromise} = this._request(url, opts); + using requestResult = this._request(url, opts); + const {req, resPromise} = requestResult; tarballChunkStream.pipe(req); tarballChunkStream.on('close', () => { req.end(); @@ -306,12 +309,13 @@ export class GitHubActionsCache implements Cache { const reqBody = JSON.stringify({ size: tarballBytes, }); - const {req, resPromise} = this._request(url, { + using requestResult = this._request(url, { method: 'POST', headers: { 'content-type': 'application/json', }, }); + const {req, resPromise} = requestResult; req.end(reqBody); const result = await resPromise; @@ -337,7 +341,7 @@ export class GitHubActionsCache implements Cache { ): { req: http.ClientRequest; resPromise: Promise>; - } { + } & Disposable { return request(url, { ...options, headers: { @@ -511,12 +515,13 @@ export class GitHubActionsCache implements Cache { version, cacheSize, }); - const {req, resPromise} = this._request(url, { + using requestResult = this._request(url, { method: 'POST', headers: { 'content-type': 'application/json', }, }); + const {req, resPromise} = requestResult; req.end(reqBody); const result = await resPromise; @@ -571,7 +576,8 @@ class GitHubActionsCacheHit implements CacheHit { } private async _download(tarballPath: string): Promise { - const {req, resPromise} = request(this._url); + using requestResult = request(this._url); + const {req, resPromise} = requestResult; req.end(); const result = await resPromise; if (!result.ok) { @@ -617,7 +623,7 @@ function request( ): { req: http.ClientRequest; resPromise: Promise>; -} { +} & Disposable { const opts = { ...options, headers: { @@ -647,7 +653,14 @@ function request( }); }, ); - return {req, resPromise}; + return { + req, + resPromise, + [Symbol.dispose]() { + req.destroy(); + req.socket?.destroy(); + }, + }; } function isOk(res: http.IncomingMessage): boolean { diff --git a/src/logging/quiet/writeover-line.ts b/src/logging/quiet/writeover-line.ts index be28c489c..14b00bc57 100644 --- a/src/logging/quiet/writeover-line.ts +++ b/src/logging/quiet/writeover-line.ts @@ -5,14 +5,7 @@ */ import {DEBUG} from '../logger.js'; - -// Quick Symbol.dispose polyfill. -if (!Symbol.dispose) { - type Writeable = {-readonly [P in keyof T]: T[P]}; - (Symbol as Writeable).dispose = Symbol( - 'dispose', - ) as typeof Symbol.dispose; -} +import '../../util/dispose.js'; export interface StatusLineWriter { clearAndStopRendering(): void; diff --git a/src/test/util/check-script-output.ts b/src/test/util/check-script-output.ts index f5776623d..1f92b98b3 100644 --- a/src/test/util/check-script-output.ts +++ b/src/test/util/check-script-output.ts @@ -38,7 +38,6 @@ export function checkScriptOutput( } } } - console.log(actual); const assertOutputEqualish = NODE_MAJOR_VERSION < 16 ? assert.match : assert.equal; assertOutputEqualish(actual, expected, message); diff --git a/src/util/dispose.ts b/src/util/dispose.ts new file mode 100644 index 000000000..6f97934b8 --- /dev/null +++ b/src/util/dispose.ts @@ -0,0 +1,15 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +// Quick Symbol.dispose polyfill. +if (!Symbol.dispose) { + type Writeable = {-readonly [P in keyof T]: T[P]}; + (Symbol as Writeable).dispose = Symbol( + 'dispose', + ) as typeof Symbol.dispose; +} + +export {}; diff --git a/src/util/fs.ts b/src/util/fs.ts index da81c97d3..0aea7259f 100644 --- a/src/util/fs.ts +++ b/src/util/fs.ts @@ -14,6 +14,7 @@ import { createWriteStream as rawCreateWriteStream, } from 'fs'; import {Deferred} from './deferred.js'; +import './dispose.js'; export {constants} from 'fs'; declare global { @@ -22,16 +23,6 @@ declare global { } } -// The interface for https://github.com/tc39/proposal-explicit-resource-management -// Once we're on TS 5.2 we can write a lot of this with `using` syntax. -interface Disposable { - [Symbol.dispose](): void; -} - -// Polyfill Symbol.dispose. -// eslint-disable-next-line -(Symbol as any).dispose ??= Symbol('Symbol.dispose'); - export class Semaphore { #remaining: number; readonly #waiting: Deferred[] = []; From 555713107126adcef0c643647150e4274f4d3dc3 Mon Sep 17 00:00:00 2001 From: Peter Burns Date: Fri, 1 Sep 2023 10:49:44 -0700 Subject: [PATCH 2/4] Downgrade our own wireit so we can test on node 20 Hilariously, because wireit 0.11.0 doesn't work with node 20, we can't test that our fixes work with node 20 without downgrading to wireit 0.10.0 for wireit's own build. 0.10.0 mostly supports node 20, though it doesn't reliably dispose of http sockets. --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index e27660e33..5dd54ce7d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -47,7 +47,7 @@ "vscode-languageclient": "^8.0.1", "vscode-languageserver": "^8.0.1", "vscode-languageserver-textdocument": "^1.0.4", - "wireit": "^0.11.0", + "wireit": "^0.10.0", "yarn": "^1.22.18" }, "engines": { @@ -6238,9 +6238,9 @@ } }, "node_modules/wireit": { - "version": "0.11.0", - "resolved": "https://registry.npmjs.org/wireit/-/wireit-0.11.0.tgz", - "integrity": "sha512-s0/bgBZ6y65BfxpCm7YA/x6QokPTQrA4mAN110FCxvvp4MotAupY8f/RH8f6tGfz74zfJafxEqHo/NyoMc0pNQ==", + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/wireit/-/wireit-0.10.0.tgz", + "integrity": "sha512-4TX6V9D/2iXUBzdqQaUG+cRePle0mDx1Q7x4Ka2cA8lgp1+ZBhrOTiLsXYRl2roQEldEFgQ2Ff1W8YgyNWAa6w==", "dev": true, "dependencies": { "braces": "^3.0.2", @@ -10955,9 +10955,9 @@ } }, "wireit": { - "version": "0.11.0", - "resolved": "https://registry.npmjs.org/wireit/-/wireit-0.11.0.tgz", - "integrity": "sha512-s0/bgBZ6y65BfxpCm7YA/x6QokPTQrA4mAN110FCxvvp4MotAupY8f/RH8f6tGfz74zfJafxEqHo/NyoMc0pNQ==", + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/wireit/-/wireit-0.10.0.tgz", + "integrity": "sha512-4TX6V9D/2iXUBzdqQaUG+cRePle0mDx1Q7x4Ka2cA8lgp1+ZBhrOTiLsXYRl2roQEldEFgQ2Ff1W8YgyNWAa6w==", "dev": true, "requires": { "braces": "^3.0.2", diff --git a/package.json b/package.json index dcf1012aa..58471b41b 100644 --- a/package.json +++ b/package.json @@ -429,7 +429,7 @@ "vscode-languageclient": "^8.0.1", "vscode-languageserver": "^8.0.1", "vscode-languageserver-textdocument": "^1.0.4", - "wireit": "^0.11.0", + "wireit": "^0.10.0", "yarn": "^1.22.18" }, "prettier": { From cf112de3ab82ae8fd5d048e05a6d64c15d77ec4d Mon Sep 17 00:00:00 2001 From: Peter Burns Date: Fri, 1 Sep 2023 10:54:27 -0700 Subject: [PATCH 3/4] Format. --- .github/workflows/tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 07c625d59..7d9d424e0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -35,7 +35,6 @@ jobs: - node: 20 os: windows-2022 - # Allow all matrix configurations to complete, instead of cancelling as # soon as one fails. Useful because we often have different kinds of # failures depending on the OS. From d5e06a0bae748472bf12417bd044d93c7a0bb31c Mon Sep 17 00:00:00 2001 From: Peter Burns Date: Fri, 1 Sep 2023 10:59:17 -0700 Subject: [PATCH 4/4] Prepare to release v0.12.0 --- CHANGELOG.md | 4 +++- package.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bde961230..9be7ea808 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] + + +## [0.12.0] - 2023-09-01 ## Added diff --git a/package.json b/package.json index 58471b41b..351f5dd6d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "wireit", - "version": "0.11.0", + "version": "0.12.0", "description": "Upgrade your npm scripts to make them smarter and more efficient", "author": "Google LLC", "license": "Apache-2.0",