From f85e89dea5e74f78d30babdc861ed26ab95c8b3f Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 3 Aug 2026 19:11:33 +0000 Subject: [PATCH 1/6] fix(core): discovery url handling, error code review helper, and pack-n-play test timeouts --- .../test/fixtures/esm-package/test/test.js | 3 ++- .../test/fixtures/leaky/test/test.ts | 3 ++- .../test/fixtures/pass/test/test.ts | 3 ++- core/dev-packages/pack-n-play/test/test.ts | 5 +++-- core/packages/gcp-metadata/src/index.ts | 22 ++++++++++++++----- .../nodejs-googleapis-common/src/discovery.ts | 12 +++++++--- .../test/test.discovery.ts | 4 ++-- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js index 037babedf227..2adbf53d289c 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js +++ b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js @@ -16,7 +16,8 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('ESM package', () => { +describe('ESM package', function () { + this.timeout(120000); it('should support esm property', () => packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts index 3a9dc7f39b39..9c56bc9c0741 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts @@ -16,7 +16,8 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('leaky tests', () => { +describe('leaky tests', function () { + this.timeout(120000); it('should fail packing n testing', async () => { await assert.rejects( packNTest({ diff --git a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts index 2dd02fb27cff..50d0d07d620d 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts @@ -15,7 +15,8 @@ import {packNTest} from 'pack-n-play'; import {describe, it} from 'mocha'; -describe('passing tests', () => { +describe('passing tests', function () { + this.timeout(120000); it('should pass the test', async () => { await packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/test.ts b/core/dev-packages/pack-n-play/test/test.ts index da954b94ba08..06aef544b4e7 100644 --- a/core/dev-packages/pack-n-play/test/test.ts +++ b/core/dev-packages/pack-n-play/test/test.ts @@ -18,7 +18,8 @@ import execa = require('execa'); import {describe, it} from 'mocha'; describe('pack-n-play', () => { - it('should run tests', async () => { + it('should run tests', async function () { + this.timeout(600000); // 10 minutes const fixturesPath = path.resolve('./test/fixtures'); const dirs = fs .readdirSync(fixturesPath) @@ -29,7 +30,7 @@ describe('pack-n-play', () => { stdio: 'inherit', cwd: dir, }; - await execa('npm', ['install'], opts); + await execa('npm', ['install', '--no-audit', '--no-fund'], opts); await execa('npm', ['link', '../../../'], opts); await execa('npm', ['test'], opts); } diff --git a/core/packages/gcp-metadata/src/index.ts b/core/packages/gcp-metadata/src/index.ts index 723fcf1d3f1e..c591ada49ef8 100644 --- a/core/packages/gcp-metadata/src/index.ts +++ b/core/packages/gcp-metadata/src/index.ts @@ -383,12 +383,24 @@ export async function isAvailable() { if (err.response && err.response.status === 404) { return false; } else { + const errObj = e as any; + const getErrorCode = (err: any): string => { + let target = err; + if ( + target instanceof Error && + target.cause && + !('code' in target) && + target.name !== 'AggregateError' + ) { + target = target.cause; + } + return target?.code ? target.code.toString() : 'UNKNOWN'; + }; + const codes = - e instanceof Error && e.name === 'AggregateError' - ? (e as any).errors.map((error: any) => - error.code ? error.code.toString() : 'UNKNOWN', - ) - : [err.code ? err.code.toString() : 'UNKNOWN']; + errObj instanceof Error && errObj.name === 'AggregateError' + ? (errObj as any).errors.map(getErrorCode) + : [getErrorCode(errObj)]; const isExpected = codes.every((code: string) => [ diff --git a/core/packages/nodejs-googleapis-common/src/discovery.ts b/core/packages/nodejs-googleapis-common/src/discovery.ts index c00c663bed28..2a665dba0291 100644 --- a/core/packages/nodejs-googleapis-common/src/discovery.ts +++ b/core/packages/nodejs-googleapis-common/src/discovery.ts @@ -13,7 +13,6 @@ import * as fs from 'fs'; import {Gaxios} from 'gaxios'; -import resolve = require('url'); import * as util from 'util'; import {GlobalOptions, ServiceOptions, APIRequestParams} from './api'; @@ -136,8 +135,15 @@ export class Discovery { apiDiscoveryUrl: string | {url?: string}, ): Promise { if (typeof apiDiscoveryUrl === 'string') { - const parts = resolve.parse(apiDiscoveryUrl); - if (apiDiscoveryUrl && !parts.protocol) { + let isUrl = false; + try { + const parsed = new URL(apiDiscoveryUrl); + isUrl = parsed.protocol === 'http:' || parsed.protocol === 'https:'; + } catch (e) { + // Not a valid URL + } + + if (apiDiscoveryUrl && !isUrl) { this.log('Reading from file ' + apiDiscoveryUrl); const file = await readFile(apiDiscoveryUrl, {encoding: 'utf8'}); return this.makeEndpoint(JSON.parse(file)); diff --git a/core/packages/nodejs-googleapis-common/test/test.discovery.ts b/core/packages/nodejs-googleapis-common/test/test.discovery.ts index 0e656c5fb6b1..6745982a218f 100644 --- a/core/packages/nodejs-googleapis-common/test/test.discovery.ts +++ b/core/packages/nodejs-googleapis-common/test/test.discovery.ts @@ -24,7 +24,7 @@ describe(__filename, () => { nock.cleanAll(); }); it('should discover an API', async () => { - const discoUrl = 'http://test.local'; + const discoUrl = 'http://test.local:80'; const scope = nock(discoUrl) .get('/') .replyWithFile(200, './test/fixtures/compute-v1.json', { @@ -37,7 +37,7 @@ describe(__filename, () => { scope.done(); }); it('should discover an API through second weird path', async () => { - const discoUrl = 'http://test.local'; + const discoUrl = 'http://test.local:80'; const scope = nock(discoUrl) .get('/') .replyWithFile(200, './test/fixtures/compute-v1.json', { From 639081b2a9dcd5ceb098a633927c4b4b60c86d6f Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 3 Aug 2026 19:42:11 +0000 Subject: [PATCH 2/6] fix(gcp-metadata): use recursive getErrorCodes helper for nested wrapper error causes --- core/packages/gcp-metadata/src/index.ts | 26 ++++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/core/packages/gcp-metadata/src/index.ts b/core/packages/gcp-metadata/src/index.ts index c591ada49ef8..3d98d38f9b03 100644 --- a/core/packages/gcp-metadata/src/index.ts +++ b/core/packages/gcp-metadata/src/index.ts @@ -384,23 +384,21 @@ export async function isAvailable() { return false; } else { const errObj = e as any; - const getErrorCode = (err: any): string => { - let target = err; - if ( - target instanceof Error && - target.cause && - !('code' in target) && - target.name !== 'AggregateError' - ) { - target = target.cause; + const getErrorCodes = (err: any): string[] => { + if (!err) return ['UNKNOWN']; + if (err.name === 'AggregateError' && Array.isArray(err.errors)) { + return err.errors.flatMap(getErrorCodes); } - return target?.code ? target.code.toString() : 'UNKNOWN'; + if (err.code) { + return [err.code.toString()]; + } + if (err.cause) { + return getErrorCodes(err.cause); + } + return ['UNKNOWN']; }; - const codes = - errObj instanceof Error && errObj.name === 'AggregateError' - ? (errObj as any).errors.map(getErrorCode) - : [getErrorCode(errObj)]; + const codes = getErrorCodes(errObj); const isExpected = codes.every((code: string) => [ From 4bc3be36f597d493f15b907a29251cc54922c38b Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 3 Aug 2026 20:08:40 +0000 Subject: [PATCH 3/6] chore(core): remove pack-n-play timeout fixes (moved to #9021) --- .../pack-n-play/test/fixtures/esm-package/test/test.js | 3 +-- .../pack-n-play/test/fixtures/leaky/test/test.ts | 3 +-- .../dev-packages/pack-n-play/test/fixtures/pass/test/test.ts | 3 +-- core/dev-packages/pack-n-play/test/test.ts | 5 ++--- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js index 2adbf53d289c..037babedf227 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js +++ b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js @@ -16,8 +16,7 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('ESM package', function () { - this.timeout(120000); +describe('ESM package', () => { it('should support esm property', () => packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts index 9c56bc9c0741..3a9dc7f39b39 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts @@ -16,8 +16,7 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('leaky tests', function () { - this.timeout(120000); +describe('leaky tests', () => { it('should fail packing n testing', async () => { await assert.rejects( packNTest({ diff --git a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts index 50d0d07d620d..2dd02fb27cff 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts @@ -15,8 +15,7 @@ import {packNTest} from 'pack-n-play'; import {describe, it} from 'mocha'; -describe('passing tests', function () { - this.timeout(120000); +describe('passing tests', () => { it('should pass the test', async () => { await packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/test.ts b/core/dev-packages/pack-n-play/test/test.ts index 06aef544b4e7..da954b94ba08 100644 --- a/core/dev-packages/pack-n-play/test/test.ts +++ b/core/dev-packages/pack-n-play/test/test.ts @@ -18,8 +18,7 @@ import execa = require('execa'); import {describe, it} from 'mocha'; describe('pack-n-play', () => { - it('should run tests', async function () { - this.timeout(600000); // 10 minutes + it('should run tests', async () => { const fixturesPath = path.resolve('./test/fixtures'); const dirs = fs .readdirSync(fixturesPath) @@ -30,7 +29,7 @@ describe('pack-n-play', () => { stdio: 'inherit', cwd: dir, }; - await execa('npm', ['install', '--no-audit', '--no-fund'], opts); + await execa('npm', ['install'], opts); await execa('npm', ['link', '../../../'], opts); await execa('npm', ['test'], opts); } From ef76bc01bb38746c27b998ea7476f91c7d6ab0fc Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 3 Aug 2026 20:17:02 +0000 Subject: [PATCH 4/6] chore(core): add pack-n-play timeout fixes back to core PR --- .../pack-n-play/test/fixtures/esm-package/test/test.js | 3 ++- .../pack-n-play/test/fixtures/leaky/test/test.ts | 3 ++- .../dev-packages/pack-n-play/test/fixtures/pass/test/test.ts | 3 ++- core/dev-packages/pack-n-play/test/test.ts | 5 +++-- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js index 037babedf227..2adbf53d289c 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js +++ b/core/dev-packages/pack-n-play/test/fixtures/esm-package/test/test.js @@ -16,7 +16,8 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('ESM package', () => { +describe('ESM package', function () { + this.timeout(120000); it('should support esm property', () => packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts index 3a9dc7f39b39..9c56bc9c0741 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/leaky/test/test.ts @@ -16,7 +16,8 @@ import {packNTest} from 'pack-n-play'; import * as assert from 'assert'; import {describe, it} from 'mocha'; -describe('leaky tests', () => { +describe('leaky tests', function () { + this.timeout(120000); it('should fail packing n testing', async () => { await assert.rejects( packNTest({ diff --git a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts index 2dd02fb27cff..50d0d07d620d 100644 --- a/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts +++ b/core/dev-packages/pack-n-play/test/fixtures/pass/test/test.ts @@ -15,7 +15,8 @@ import {packNTest} from 'pack-n-play'; import {describe, it} from 'mocha'; -describe('passing tests', () => { +describe('passing tests', function () { + this.timeout(120000); it('should pass the test', async () => { await packNTest({ sample: { diff --git a/core/dev-packages/pack-n-play/test/test.ts b/core/dev-packages/pack-n-play/test/test.ts index da954b94ba08..06aef544b4e7 100644 --- a/core/dev-packages/pack-n-play/test/test.ts +++ b/core/dev-packages/pack-n-play/test/test.ts @@ -18,7 +18,8 @@ import execa = require('execa'); import {describe, it} from 'mocha'; describe('pack-n-play', () => { - it('should run tests', async () => { + it('should run tests', async function () { + this.timeout(600000); // 10 minutes const fixturesPath = path.resolve('./test/fixtures'); const dirs = fs .readdirSync(fixturesPath) @@ -29,7 +30,7 @@ describe('pack-n-play', () => { stdio: 'inherit', cwd: dir, }; - await execa('npm', ['install'], opts); + await execa('npm', ['install', '--no-audit', '--no-fund'], opts); await execa('npm', ['link', '../../../'], opts); await execa('npm', ['test'], opts); } From ac1f4781c303d05946bba244f9dc8c18d07339bf Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Mon, 3 Aug 2026 21:07:52 +0000 Subject: [PATCH 5/6] add tests for error.cause and AggregateError --- core/packages/gcp-metadata/test/index.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/core/packages/gcp-metadata/test/index.test.ts b/core/packages/gcp-metadata/test/index.test.ts index 83517852b622..f88a018efffa 100644 --- a/core/packages/gcp-metadata/test/index.test.ts +++ b/core/packages/gcp-metadata/test/index.test.ts @@ -493,6 +493,45 @@ describe('unit test', () => { }); }); + it('should fail on isAvailable if ENOTFOUND is wrapped in error.cause', async () => { + const secondary = secondaryHostRequest(500, 'ENOTFOUND'); + const innerErr = Object.assign(new Error('ENOTFOUND'), {code: 'ENOTFOUND'}); + const wrapperErr = Object.assign(new Error('Wrapper error'), { + cause: innerErr, + }); + const primary = nock(HOST) + .get(`${PATH}/${TYPE}`) + .replyWithError(wrapperErr); + const isGCE = await gcp.isAvailable(); + await secondary; + primary.done(); + assert.strictEqual(false, isGCE); + }); + + it('should fail on isAvailable if ENOTFOUND is wrapped inside an AggregateError or nested cause', async () => { + const secondary = secondaryHostRequest(500, 'ENOTFOUND'); + const innerErr1 = Object.assign(new Error('ENOTFOUND'), { + code: 'ENOTFOUND', + }); + const innerErr2 = Object.assign(new Error('EHOSTUNREACH'), { + code: 'EHOSTUNREACH', + }); + const wrapperErr = Object.assign(new Error('Wrapper error'), { + cause: innerErr1, + }); + const aggregateErr = new AggregateError( + [wrapperErr, innerErr2], + 'Aggregate error', + ); + const primary = nock(HOST) + .get(`${PATH}/${TYPE}`) + .replyWithError(aggregateErr); + const isGCE = await gcp.isAvailable(); + await secondary; + primary.done(); + assert.strictEqual(false, isGCE); + }); + it('should return first successful response', async () => { const secondary = secondaryHostRequest(500); const primary = nock(HOST).get(`${PATH}/${TYPE}`).reply(404); From 4339fd166f94196a0ce8d2fb373a8d1406d4be76 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Wed, 5 Aug 2026 12:02:07 +0000 Subject: [PATCH 6/6] revert unnecessary addition of port --- core/packages/nodejs-googleapis-common/test/test.discovery.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/packages/nodejs-googleapis-common/test/test.discovery.ts b/core/packages/nodejs-googleapis-common/test/test.discovery.ts index 6745982a218f..0e656c5fb6b1 100644 --- a/core/packages/nodejs-googleapis-common/test/test.discovery.ts +++ b/core/packages/nodejs-googleapis-common/test/test.discovery.ts @@ -24,7 +24,7 @@ describe(__filename, () => { nock.cleanAll(); }); it('should discover an API', async () => { - const discoUrl = 'http://test.local:80'; + const discoUrl = 'http://test.local'; const scope = nock(discoUrl) .get('/') .replyWithFile(200, './test/fixtures/compute-v1.json', { @@ -37,7 +37,7 @@ describe(__filename, () => { scope.done(); }); it('should discover an API through second weird path', async () => { - const discoUrl = 'http://test.local:80'; + const discoUrl = 'http://test.local'; const scope = nock(discoUrl) .get('/') .replyWithFile(200, './test/fixtures/compute-v1.json', {