From 917c6cbad176f1464cc596f8f01f805c93ac3dfe Mon Sep 17 00:00:00 2001 From: Brendan Burns <5751682+brendandburns@users.noreply.github.com> Date: Wed, 12 Feb 2025 15:33:41 +0000 Subject: [PATCH 1/2] Fix a bug where an error on request wouldn't propogate. Add tests. --- src/cache.ts | 12 +++++++++--- src/cache_test.ts | 27 ++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/cache.ts b/src/cache.ts index 8b580d02ed..2ee8c4f80c 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -10,7 +10,7 @@ import { ObjectCallback, UPDATE, } from './informer.js'; -import { KubernetesObject } from './types.js'; +import { KubernetesObject, KubernetesListObject } from './types.js'; import { ObjectSerializer } from './serializer.js'; import { Watch } from './watch.js'; @@ -143,8 +143,14 @@ export class ListWatch implements ObjectCache, In } this.callbackCache[CONNECT].forEach((elt: ErrorCallback) => elt(undefined)); if (!this.resourceVersion) { - const promise = this.listFn(); - const list = await promise; + let list: KubernetesListObject; + try { + const promise = this.listFn(); + list = await promise; + } catch (err) { + this.callbackCache[ERROR].forEach((elt: ErrorCallback) => elt(err)); + return; + } this.objects = deleteItems(this.objects, list.items, this.callbackCache[DELETE].slice()); this.addOrUpdateItems(list.items); this.resourceVersion = list.metadata!.resourceVersion || ''; diff --git a/src/cache_test.ts b/src/cache_test.ts index bfcd7114f3..b730b3b994 100644 --- a/src/cache_test.ts +++ b/src/cache_test.ts @@ -1,4 +1,4 @@ -import { deepStrictEqual, notStrictEqual, strictEqual, throws } from 'node:assert'; +import { deepStrictEqual, fail, equal, notStrictEqual, strictEqual, throws } from 'node:assert'; import mock from 'ts-mockito'; import { V1Namespace, V1NamespaceList, V1ObjectMeta, V1Pod, V1PodList, V1ListMeta } from './api.js'; @@ -1460,4 +1460,29 @@ describe('delete items', () => { strictEqual(await connectPromise, true); }); + + it('should correctly handle errors in the initial list', async () => { + const fake = mock.mock(Watch); + const requestErr = Error('request failed'); + const listFn: ListPromise = function (): Promise { + return new Promise((resolve, reject) => { + reject(requestErr); + }); + }; + const lw = new ListWatch('/some/path', fake, listFn); + let gotErr: Error | null = null; + const errCalled = new Promise((resolve, reject) => { + lw.on('error', (err) => { + gotErr = err; + resolve(); + }); + }); + try { + await lw.start(); + await errCalled; + equal(gotErr, requestErr); + } catch (err) { + fail(`unexpected error: ${err}`); + } + }); }); From 8632d50c7d9aa00169d31015a6df8efdf41e9311 Mon Sep 17 00:00:00 2001 From: Brendan Burns <5751682+brendandburns@users.noreply.github.com> Date: Fri, 14 Feb 2025 18:42:56 +0000 Subject: [PATCH 2/2] Address comments --- src/cache_test.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cache_test.ts b/src/cache_test.ts index b730b3b994..e2e0123818 100644 --- a/src/cache_test.ts +++ b/src/cache_test.ts @@ -1,4 +1,4 @@ -import { deepStrictEqual, fail, equal, notStrictEqual, strictEqual, throws } from 'node:assert'; +import { deepStrictEqual, notStrictEqual, strictEqual, throws } from 'node:assert'; import mock from 'ts-mockito'; import { V1Namespace, V1NamespaceList, V1ObjectMeta, V1Pod, V1PodList, V1ListMeta } from './api.js'; @@ -1477,12 +1477,8 @@ describe('delete items', () => { resolve(); }); }); - try { - await lw.start(); - await errCalled; - equal(gotErr, requestErr); - } catch (err) { - fail(`unexpected error: ${err}`); - } + await lw.start(); + await errCalled; + strictEqual(gotErr, requestErr); }); });