diff --git a/.chronus/changes/copilot-fix-global-policy-error-2026-03-04.md b/.chronus/changes/copilot-fix-global-policy-error-2026-03-04.md new file mode 100644 index 00000000000..eb2eed1f4c7 --- /dev/null +++ b/.chronus/changes/copilot-fix-global-policy-error-2026-03-04.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-server-js" +--- + +Fix `createPolicyChain` to return a `Promise` when policies are specified, preventing `TypeError: Cannot read properties of undefined (reading 'catch')` when using global policies. diff --git a/packages/http-server-js/generated-defs/helpers/router.ts b/packages/http-server-js/generated-defs/helpers/router.ts index 12ac11cc4c7..98f5b1994ca 100644 --- a/packages/http-server-js/generated-defs/helpers/router.ts +++ b/packages/http-server-js/generated-defs/helpers/router.ts @@ -53,28 +53,34 @@ const lines = [ " return out;", " }", "", - " function applyPolicy(ctx: HttpContext, index: number) {", + " function applyPolicy(ctx: HttpContext, index: number): Promise {", " if (index >= policies.length) {", - " return out(ctx, ...outParams);", + " return Promise.resolve(out(ctx, ...outParams));", " }", "", - " policies[index](ctx, function nextPolicy(nextRequest) {", - " applyPolicy(", - " {", - " ...ctx,", - " request: nextRequest ?? ctx.request,", - " },", - " index + 1,", - " );", + " return new Promise((resolve, reject) => {", + " try {", + " policies[index](ctx, function nextPolicy(nextRequest) {", + " applyPolicy(", + " {", + " ...ctx,", + " request: nextRequest ?? ctx.request,", + " },", + " index + 1,", + " ).then(resolve, reject);", + " });", + " } catch (e) {", + " reject(e);", + " }", " });", " }", "", " return {", " [name](ctx: HttpContext, ...params: any[]) {", " outParams = params;", - " applyPolicy(ctx, 0);", + " return applyPolicy(ctx, 0);", " },", - " }[name] as Out;", + " }[name] as unknown as Out;", "}", "", "/**", diff --git a/packages/http-server-js/src/helpers/router.ts b/packages/http-server-js/src/helpers/router.ts index e07d765266b..e2c04f890f9 100644 --- a/packages/http-server-js/src/helpers/router.ts +++ b/packages/http-server-js/src/helpers/router.ts @@ -44,28 +44,34 @@ export function createPolicyChain { if (index >= policies.length) { - return out(ctx, ...outParams); + return Promise.resolve(out(ctx, ...outParams)); } - policies[index](ctx, function nextPolicy(nextRequest) { - applyPolicy( - { - ...ctx, - request: nextRequest ?? ctx.request, - }, - index + 1, - ); + return new Promise((resolve, reject) => { + try { + policies[index](ctx, function nextPolicy(nextRequest) { + applyPolicy( + { + ...ctx, + request: nextRequest ?? ctx.request, + }, + index + 1, + ).then(resolve, reject); + }); + } catch (e) { + reject(e); + } }); } return { [name](ctx: HttpContext, ...params: any[]) { outParams = params; - applyPolicy(ctx, 0); + return applyPolicy(ctx, 0); }, - }[name] as Out; + }[name] as unknown as Out; } /** diff --git a/packages/http-server-js/test/router.test.ts b/packages/http-server-js/test/router.test.ts new file mode 100644 index 00000000000..2d2014b8a89 --- /dev/null +++ b/packages/http-server-js/test/router.test.ts @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation +// Licensed under the MIT license. + +import { describe, expect, it } from "vitest"; +import { createPolicyChain, type HttpContext, type Policy } from "../src/helpers/router.js"; +import type * as http from "node:http"; + +function makeContext(): HttpContext { + return { + request: {} as http.IncomingMessage, + response: {} as http.ServerResponse, + errorHandlers: { + onRequestNotFound: () => {}, + onInvalidRequest: () => {}, + onInternalError: () => {}, + }, + }; +} + +describe("createPolicyChain", () => { + it("returns the out function directly when no policies are provided", () => { + const out = async (_ctx: HttpContext) => {}; + const chain = createPolicyChain("test", [], out); + expect(chain).toBe(out); + }); + + it("returns a Promise when policies are specified (fixes TypeError on .catch)", async () => { + const policy: Policy = async (ctx, next) => { + next(); + }; + const out = async (_ctx: HttpContext) => {}; + const chain = createPolicyChain("test", [policy], out); + const ctx = makeContext(); + const result = chain(ctx); + // Must be a Promise (so .catch() doesn't throw TypeError) + expect(result).toBeInstanceOf(Promise); + await result; + }); + + it("calls the out function after a single policy", async () => { + const calls: string[] = []; + const policy: Policy = async (ctx, next) => { + calls.push("policy"); + next(); + }; + const out = async (_ctx: HttpContext) => { + calls.push("out"); + }; + const chain = createPolicyChain("test", [policy], out); + await chain(makeContext()); + expect(calls).toEqual(["policy", "out"]); + }); + + it("calls policies in order before calling out", async () => { + const calls: string[] = []; + const policy1: Policy = (ctx, next) => { + calls.push("policy1"); + next(); + }; + const policy2: Policy = (ctx, next) => { + calls.push("policy2"); + next(); + }; + const out = async (_ctx: HttpContext) => { + calls.push("out"); + }; + const chain = createPolicyChain("test", [policy1, policy2], out); + await chain(makeContext()); + expect(calls).toEqual(["policy1", "policy2", "out"]); + }); + + it("propagates errors from out through the returned Promise", async () => { + const policy: Policy = (ctx, next) => { + next(); + }; + const out = async (_ctx: HttpContext) => { + throw new Error("test error"); + }; + const chain = createPolicyChain("test", [policy], out); + await expect(chain(makeContext())).rejects.toThrow("test error"); + }); + + it("propagates errors thrown synchronously in a policy", async () => { + const policy: Policy = (_ctx, _next) => { + throw new Error("policy error"); + }; + const out = async (_ctx: HttpContext) => {}; + const chain = createPolicyChain("test", [policy], out); + await expect(chain(makeContext())).rejects.toThrow("policy error"); + }); + + it("passes updated request from policy to out", async () => { + let receivedRequest: http.IncomingMessage | undefined; + const newRequest = { url: "/updated" } as http.IncomingMessage; + const policy: Policy = (ctx, next) => { + next(newRequest); + }; + const out = async (ctx: HttpContext) => { + receivedRequest = ctx.request; + }; + const chain = createPolicyChain("test", [policy], out); + await chain(makeContext()); + expect(receivedRequest).toBe(newRequest); + }); +});