From 5bc4add09944da86f4a2e66e0a7e906ac1f284ae Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 17 May 2026 05:06:58 -0400 Subject: [PATCH] refactor(mesh): retarget proxyFetch null-target throw to no_target (#1083) `MeshService.proxyFetch` previously threw `MeshError('push_failed', ...)` when `NodeRegistry.getProxyTarget(nodeId)` returned null, but semantically no push was attempted: the target simply did not exist. Callers had to special-case the code and explain the overload in a comment. Retarget the throw to the existing `'no_target'` variant in `MeshErrorCode`. `listStacksOnNode` matches the new code directly; the explanatory comment is gone since the code reads for itself. `MeshError('push_failed', ...)` continues to signal real outbound push failures: `applyLocalOverride` (mesh data plane not yet ready on the receiving node) and `pushOverrideToNode` (HTTP 404 from an outdated remote, non-OK HTTP response). Those sites are unchanged. Operator-visible behavior is unchanged in the steady state. The rare "remote disappeared between opt-in start and override push" case now returns HTTP 400 with `code: 'no_target'` instead of 503; the response body carries a clear message ("no proxy target for node N") so clients can disambiguate. Test plan - `npx tsc --noEmit`: clean. - `npx vitest run src/__tests__/mesh-list-stacks-remote.test.ts`: 8/8 green (6 existing + 2 new cases pinning the catch-arm semantics). - `npx vitest run src/__tests__/mesh-service.test.ts src/__tests__/mesh-inspect-remote.test.ts`: 56/56 green. - Code reviewed; findings applied. Notes - PR #1082 (refactor: route inspectStackServices through proxyFetch) needs a rebase + one-line update to its catch arm before merging on top of this change. --- .../__tests__/mesh-list-stacks-remote.test.ts | 63 ++++++++++++++++++- backend/src/services/MeshService.ts | 7 +-- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/backend/src/__tests__/mesh-list-stacks-remote.test.ts b/backend/src/__tests__/mesh-list-stacks-remote.test.ts index 1f75596a..c265bd0e 100644 --- a/backend/src/__tests__/mesh-list-stacks-remote.test.ts +++ b/backend/src/__tests__/mesh-list-stacks-remote.test.ts @@ -17,13 +17,14 @@ import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb'; let tmpDir: string; let MeshService: typeof import('../services/MeshService').MeshService; +let MeshError: typeof import('../services/MeshService').MeshError; let DatabaseService: typeof import('../services/DatabaseService').DatabaseService; let NodeRegistry: typeof import('../services/NodeRegistry').NodeRegistry; let FileSystemService: typeof import('../services/FileSystemService').FileSystemService; beforeAll(async () => { tmpDir = await setupTestDb(); - ({ MeshService } = await import('../services/MeshService')); + ({ MeshService, MeshError } = await import('../services/MeshService')); ({ DatabaseService } = await import('../services/DatabaseService')); ({ NodeRegistry } = await import('../services/NodeRegistry')); ({ FileSystemService } = await import('../services/FileSystemService')); @@ -179,4 +180,64 @@ describe('MeshService.listStacksOnNode dispatch (F8)', () => { expect(out).toEqual([]); expect(fetchSpy).not.toHaveBeenCalled(); }); + + it('catch arm swallows MeshError(no_target) and logs the no-proxy-target warning', async () => { + const svc = MeshService.getInstance(); + const db = DatabaseService.getInstance(); + const remoteNodeId = db.addNode({ + name: 'list-stacks-no-target-code', + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/tmp', + is_default: false, + api_url: '', + api_token: '', + }); + + vi.spyOn(NodeRegistry.getInstance(), 'getProxyTarget').mockReturnValue(null); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const out = await svc.listStacksOnNode(remoteNodeId); + + expect(out).toEqual([]); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(String(warnSpy.mock.calls[0][0])).toContain('no proxy target for node'); + expect(errorSpy).not.toHaveBeenCalled(); + + db.deleteNode(remoteNodeId); + }); + + it('unexpected MeshError codes fall through to the remote-unreachable error path', async () => { + const svc = MeshService.getInstance(); + const db = DatabaseService.getInstance(); + const remoteNodeId = db.addNode({ + name: 'list-stacks-push-failed-falls-through', + type: 'remote', + mode: 'proxy', + compose_dir: '/tmp', + is_default: false, + api_url: 'https://remote.example.com:1852', + api_token: 'remote-tok', + }); + + vi.spyOn(NodeRegistry.getInstance(), 'getProxyTarget').mockReturnValue({ + apiUrl: 'https://remote.example.com:1852', + apiToken: 'remote-tok', + }); + vi.spyOn(globalThis, 'fetch').mockImplementation(() => { + throw new MeshError('push_failed', 'simulated transport failure'); + }); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const out = await svc.listStacksOnNode(remoteNodeId); + + expect(out).toEqual([]); + expect(warnSpy).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(String(errorSpy.mock.calls[0][0])).toContain('remote unreachable'); + + db.deleteNode(remoteNodeId); + }); }); diff --git a/backend/src/services/MeshService.ts b/backend/src/services/MeshService.ts index cddccff1..d4bf0fdc 100644 --- a/backend/src/services/MeshService.ts +++ b/backend/src/services/MeshService.ts @@ -1233,10 +1233,7 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { if (!Array.isArray(body.stacks)) return []; return body.stacks.filter((s): s is string => typeof s === 'string'); } catch (err) { - // proxyFetch throws MeshError('push_failed') when getProxyTarget - // returns null (e.g. pilot tunnel offline). Treat the same as a - // non-OK response: empty list. - if (err instanceof MeshError && err.code === 'push_failed') { + if (err instanceof MeshError && err.code === 'no_target') { console.warn(`[MeshService] listStacksOnNode: no proxy target for node ${nodeId} (${sanitizeForLog(node.name)})`); return []; } @@ -1263,7 +1260,7 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { timeoutMs: number, ): Promise { const target = NodeRegistry.getInstance().getProxyTarget(nodeId); - if (!target) throw new MeshError('push_failed', `no proxy target for node ${nodeId}`); + if (!target) throw new MeshError('no_target', `no proxy target for node ${nodeId}`); const url = `${target.apiUrl.replace(/\/$/, '')}${apiPath}`; const headers: Record = {}; if (body !== undefined) headers['Content-Type'] = 'application/json';