diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml index 2d0d1b2..cf5948d 100644 --- a/.github/workflows/manual-release.yml +++ b/.github/workflows/manual-release.yml @@ -34,7 +34,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index d1a5ac3..6777e16 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -11,10 +11,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -78,10 +78,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -105,10 +105,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -140,10 +140,10 @@ jobs: aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ secrets.AWS_REGION }} - - name: Use Node.js 22 + - name: Use Node.js 24 uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 cache: 'npm' - name: Install dependencies run: npm ci @@ -215,12 +215,18 @@ jobs: name: "Package Verification" runs-on: ubuntu-latest needs: [test] + # Run across the full matrix: packaging is version-sensitive (ESM + # resolution and install/import behavior differ between Node versions), + # and this job needs no AWS credentials, so the extra coverage is cheap. + strategy: + matrix: + node-version: [22, 24, 26] steps: - uses: actions/checkout@v4 - - name: Use Node.js 22 + - name: Use Node.js ${{ matrix.node-version }} uses: actions/setup-node@v4 with: - node-version: 22 + node-version: ${{ matrix.node-version }} cache: 'npm' - name: Install dependencies run: npm ci diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 681b8a1..2b3c190 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,7 +26,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: 22 + node-version: 24 registry-url: 'https://registry.npmjs.org' cache: 'npm' diff --git a/test/streaming-memory.test.ts b/test/streaming-memory.test.ts index 6fae3ae..743f1fc 100644 --- a/test/streaming-memory.test.ts +++ b/test/streaming-memory.test.ts @@ -1,16 +1,35 @@ import { Readable, Writable } from 'node:stream'; +import v8 from 'node:v8'; +import vm from 'node:vm'; import { GetObjectCommand, HeadBucketCommand, S3Client } from '@aws-sdk/client-s3'; import { mockClient } from 'aws-sdk-client-mock'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { S3Proxy } from '../src/index.js'; import { makeReq } from './helpers/http-mocks.js'; +// Obtain a GC trigger without requiring the runner to launch with --expose-gc. +// Vitest does not reliably forward `--expose-gc` to its worker forks, so expose +// gc programmatically via V8 flags instead. Returns null if unavailable (the +// test then skips the strict bound rather than false-failing on uncollected +// garbage). +function acquireGc(): (() => void) | null { + if (typeof global.gc === 'function') return global.gc.bind(global); + try { + v8.setFlagsFromString('--expose-gc'); + const gc = vm.runInNewContext('gc') as (() => void) | undefined; + v8.setFlagsFromString('--no-expose-gc'); + return typeof gc === 'function' ? gc : null; + } catch { + return null; + } +} + // Runs in the plain `test:unit` pass only — `test:coverage` excludes this file -// (see package.json). The assertion measures process RSS, and V8 coverage -// instrumentation inflates RSS and shifts GC timing enough to push the peak -// delta past the bound (~57-62MB observed vs. the 50MB limit), producing false -// failures that have nothing to do with s3proxy's buffering. Un-instrumented, -// the measurement is stable and meaningful, so the strict bound lives here. +// (see package.json). The assertion samples memory usage, and V8 coverage +// instrumentation perturbs allocation and GC timing enough to make the +// measurement unreliable, producing failures that have nothing to do with +// s3proxy's buffering. Un-instrumented, the measurement is stable and +// meaningful, so the bound lives here. describe('streaming memory bound', () => { const s3Mock = mockClient(S3Client); @@ -25,7 +44,7 @@ describe('streaming memory bound', () => { s3Mock.restore(); }); - it('streams a 100MB body with peak RSS delta < 50MB', async () => { + it('streams a 100MB body without buffering the full payload', async () => { const SIZE = 100 * 1024 * 1024; const CHUNK = 64 * 1024; let remaining = SIZE; @@ -51,10 +70,22 @@ describe('streaming memory bound', () => { const proxy = new S3Proxy({ bucket: 'streaming-test' }); await proxy.init(); - if (typeof global.gc === 'function') global.gc(); - const baseRss = process.memoryUsage().rss; + // Force a full GC every GC_WINDOW chunks so the sampled peak reflects the + // *retained* working set rather than uncollected transient garbage. + const gc = acquireGc(); + const GC_WINDOW = 16; // ~1MB of 64KB chunks between forced collections + + gc?.(); + // Measure external Buffer memory (arrayBuffers), not RSS: the payload lives + // in external Buffer storage, not the JS heap, and RSS is a sticky + // high-water mark that V8 does not return to the OS after GC — which is why + // an RSS bound drifts between Node versions. arrayBuffers drops the instant + // freed buffers are collected, so with periodic forced GC it tracks what + // s3proxy actually holds. + const baseAb = process.memoryUsage().arrayBuffers; let peakDelta = 0; let consumed = 0; + let i = 0; const { stream } = await proxy.fetch(makeReq('/big.bin')); @@ -62,7 +93,8 @@ describe('streaming memory bound', () => { const sink = new Writable({ write(chunk: Buffer, _enc, cb) { consumed += chunk.length; - const delta = process.memoryUsage().rss - baseRss; + if (gc && ++i % GC_WINDOW === 0) gc(); + const delta = process.memoryUsage().arrayBuffers - baseAb; if (delta > peakDelta) peakDelta = delta; cb(); }, @@ -74,6 +106,18 @@ describe('streaming memory bound', () => { }); expect(consumed).toBe(SIZE); - expect(peakDelta).toBeLessThan(50 * 1024 * 1024); + // The invariant under test is that s3proxy streams rather than buffering + // the full body. Under backpressure the pipeline retains only a handful of + // in-flight chunks (~1-2MB observed, dominated by the 1MB GC window); a + // buffering implementation would keep the whole body referenced, so forced + // GC could not reclaim it and the delta would climb toward SIZE (100MB). + // The 8MB bound sits far above the streaming working set and far below a + // buffering regression, and — because the GC cadence is forced rather than + // left to the runtime — holds steady across Node versions. If GC could not + // be acquired, transient garbage never gets collected and the measurement + // is meaningless, so the strict bound is skipped. + if (gc) { + expect(peakDelta).toBeLessThan(8 * 1024 * 1024); + } }); }); diff --git a/vitest.config.ts b/vitest.config.ts index 3655a12..9ea7c1a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -18,8 +18,5 @@ export default defineConfig({ }, }, pool: 'forks', - forks: { - execArgv: ['--expose-gc'], - }, }, });