From 95d9b95d132377d89ce455ccce05530329cdf5bd Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Mon, 11 May 2026 03:19:26 +0000 Subject: [PATCH] feat(unified-callhandler): simplify CallHandler to delegate to registry.execute() - Remove separate spec lookup, handler lookup, access control, and input validation from buildCallHandler - Call registry.execute() directly; access control enforced via execute() (trusted not set) - On error, look up spec for errorSchemas and pass to mapError() - Make callMap required in CallHandlerConfig (no longer optional) - Update tests: remove no-callMap tests, use callMap for all handler tests - Add test for mapError with spec errorSchemas - All 226 tests passing --- src/call.ts | 16 +-- test/call.test.ts | 253 +++++++++++++++++++++++++++------------------- 2 files changed, 156 insertions(+), 113 deletions(-) diff --git a/src/call.ts b/src/call.ts index cc8f53b..c8cbc2d 100644 --- a/src/call.ts +++ b/src/call.ts @@ -58,7 +58,7 @@ interface PendingRequest { export interface CallHandlerConfig { registry: OperationRegistry; - callMap?: PendingRequestMap; + callMap: PendingRequestMap; } export type CallHandler = (event: CallRequestedEvent) => Promise; @@ -194,17 +194,11 @@ export function buildCallHandler(config: CallHandlerConfig): CallHandler { try { const envelope = await registry.execute(operationId, input, context); - - if (callMap) { - callMap.respond(requestId, envelope); - } + callMap.respond(requestId, envelope); } catch (error) { - const callError = mapError(error); - if (callMap) { - callMap.emitError(requestId, callError.code, callError.message, callError.details); - } else { - throw callError; - } + const spec = registry.getSpec(operationId); + const callError = mapError(error, spec?.errorSchemas); + callMap.emitError(requestId, callError.code, callError.message, callError.details); } }; } diff --git a/test/call.test.ts b/test/call.test.ts index 9a07b6b..357b306 100644 --- a/test/call.test.ts +++ b/test/call.test.ts @@ -512,66 +512,26 @@ describe("CallHandler", () => { } }); - it("throws error when no callMap and operation not found", async () => { + it("publishes call.error for unknown operations with mapError defaults", async () => { const registry = new OperationRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); - await expect( - handler({ - requestId: "r1", - operationId: "test.nonexistent", - input: {}, - }), - ).rejects.toThrow("Operation not found"); - }); + const callPromise = callMap.call("test.nonexistent", {}); - it("throws error when no callMap and access denied", async () => { - const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); - - const identity: Identity = { id: "user1", scopes: [] }; - - await expect( - handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }), - ).rejects.toThrow("Access denied"); - }); - - it("works without callMap for successful operations", async () => { - const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); - - const identity: Identity = { - id: "user1", - scopes: [], - resources: { "project:abc": ["read"] }, - }; - - const result = await handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }); - - expect(result).toBeUndefined(); - }); - - it("works without callMap for open operations", async () => { - const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); - - const result = await handler({ - requestId: "r1", - operationId: "test.open", + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.nonexistent", input: {}, }); - expect(result).toBeUndefined(); + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe(InfrastructureErrorCode.OPERATION_NOT_FOUND); + } }); it("applies Value.Cast normalization via execute()", async () => { @@ -631,6 +591,44 @@ describe("CallHandler", () => { const result = await callPromise; expect(result.data).toEqual({ name: "test", extra: "field" }); }); + + it("maps errors using spec errorSchemas", async () => { + const registry = new OperationRegistry(); + registry.register({ + name: "customError", + namespace: "test", + version: "1.0.0", + type: OperationType.QUERY, + description: "op with custom errors", + inputSchema: Type.Object({}), + outputSchema: Type.Unknown(), + accessControl: { requiredScopes: [] }, + errorSchemas: [ + { code: "NOT_FOUND", description: "Item not found", schema: Type.Object({ id: Type.String() }) }, + ], + handler: async () => { throw new Error("NOT_FOUND: item 42"); }, + }); + + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); + + const callPromise = callMap.call("test.customError", {}); + + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.customError", + input: {}, + }); + + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe("NOT_FOUND"); + expect((error as CallError).message).toBe("NOT_FOUND: item 42"); + } + }); }); describe("checkAccess resource access control", () => { @@ -670,39 +668,58 @@ describe("checkAccess resource access control", () => { it("denies access when resourceType/resourceAction are set and identity.resources is undefined", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", scopes: [] }; - await expect( - handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }), - ).rejects.toThrow("Access denied"); + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.guarded", + input: {}, + identity, + }); + + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe(InfrastructureErrorCode.ACCESS_DENIED); + } }); it("denies access when resourceType/resourceAction are set and identity.resources is empty", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", scopes: [], resources: {} }; - await expect( - handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }), - ).rejects.toThrow("Access denied"); + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.guarded", + input: {}, + identity, + }); + + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe(InfrastructureErrorCode.ACCESS_DENIED); + } }); it("denies access when identity.resources has no matching resource type", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", @@ -710,19 +727,28 @@ describe("checkAccess resource access control", () => { resources: { "document:abc": ["read"] }, }; - await expect( - handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }), - ).rejects.toThrow("Access denied"); + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.guarded", + input: {}, + identity, + }); + + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe(InfrastructureErrorCode.ACCESS_DENIED); + } }); it("denies access when identity.resources has matching type but wrong action", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", @@ -730,19 +756,28 @@ describe("checkAccess resource access control", () => { resources: { "project:abc": ["write"] }, }; - await expect( - handler({ - requestId: "r1", - operationId: "test.guarded", - input: {}, - identity, - }), - ).rejects.toThrow("Access denied"); + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], + operationId: "test.guarded", + input: {}, + identity, + }); + + try { + await callPromise; + expect.fail("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(CallError); + expect((error as CallError).code).toBe(InfrastructureErrorCode.ACCESS_DENIED); + } }); it("grants access when identity.resources has matching type and action", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", @@ -750,33 +785,43 @@ describe("checkAccess resource access control", () => { resources: { "project:abc": ["read"] }, }; - const result = await handler({ - requestId: "r1", + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], operationId: "test.guarded", input: {}, identity, }); - expect(result).toBeUndefined(); + + const result = await callPromise; + expect(result.data).toEqual({ ok: true }); }); it("grants access when neither resourceType nor resourceAction are set", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", scopes: [] }; - const result = await handler({ - requestId: "r1", + const callPromise = callMap.call("test.open", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], operationId: "test.open", input: {}, identity, }); - expect(result).toBeUndefined(); + + const result = await callPromise; + expect(result.data).toEqual({ ok: true }); }); it("grants access when identity.resources matches and identity has no scopes required", async () => { const registry = makeRegistry(); - const handler = buildCallHandler({ registry }); + const callMap = new PendingRequestMap(); + const handler = buildCallHandler({ registry, callMap }); const identity: Identity = { id: "user1", @@ -784,12 +829,16 @@ describe("checkAccess resource access control", () => { resources: { "project:xyz": ["read", "write"] }, }; - const result = await handler({ - requestId: "r1", + const callPromise = callMap.call("test.guarded", {}, { identity }); + + await handler({ + requestId: [...callMap["requests"].keys()][0], operationId: "test.guarded", input: {}, identity, }); - expect(result).toBeUndefined(); + + const result = await callPromise; + expect(result.data).toEqual({ ok: true }); }); }); \ No newline at end of file