fix(compiler): stop error {} from swallowing response {} bugs in api blocks
Client codegen chained .then().catch(), so a .catch() after .then() caught exceptions thrown by the response body too. Switched to the two-argument then(onFulfilled, onRejected) form, whose rejection handler cannot see errors from the fulfilment handler. SSR codegen wrapped both the transport call and the response-body eval in the same try; only __wrnexusCallApi is now inside the try, and __wrnexusEvalData runs after it, outside. Also verifies (and locks in with a regression test) that GET query numbers already coerce correctly through defineEndpoint + checkField, and documents that in the typed-api-block spec.
This commit is contained in:
@@ -158,6 +158,22 @@ A plain handler returning `Response.json` has no `defineEndpoint` contract, so `
|
||||
to `unknown`. The block's declared types are used directly and the generator emits a warning naming
|
||||
the route. Untyped endpoints stay visible rather than silently passing.
|
||||
|
||||
### GET parameters travel as strings
|
||||
|
||||
A `GET` block's `parameters` become a query string (see Request assembly below), and every
|
||||
`URLSearchParams` value is text on the wire regardless of the declared field type — a block
|
||||
declaring `age?: number` still sends and receives `"30"`, not `30`. The declared type is honest
|
||||
only because the endpoint's own schema coerces it back: `checkField` in
|
||||
`packages/validation/src/index.ts` calls `Number(pre)` for every `v.number()` field — optional or
|
||||
required — before the handler ever sees it, so `defineEndpoint({ input: v.object({ age:
|
||||
v.number() }) })` invoked as `?age=30` hands the handler an actual `number` (verified end to end;
|
||||
regression-tested in `packages/core/test/endpoint-schema.test.ts`, "a GET request coerces a
|
||||
v.number() query param to an actual number"). This is a property of the endpoint's schema, not of
|
||||
the `api` block or the generated contract types — a route that reads `ctx.url.searchParams`
|
||||
directly, with no `defineEndpoint` schema, receives raw strings and gets no coercion, but that
|
||||
route also has no contract for the generator to check against, so it already falls under "Routes
|
||||
without a contract" above and is flagged there.
|
||||
|
||||
### Staleness
|
||||
|
||||
Checking is only as current as the generated file, so this stays wired into the existing
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use strict";
|
||||
// Generated by scripts/build-editor-compiler.mjs. Do not edit directly.
|
||||
// WRN editor compiler source hash: fde135d610b595588974c7b88dc73ddf10c1da99e106a2c0a009be2707fc13a8
|
||||
// WRN editor compiler source hash: b0e3094d8c2a70ee2b34fe961c186b58de9527aa072ca12292b715d3d5f51c87
|
||||
// WRN editor compiler generator hash: a54ca847c758bc98d8e353ad6d70088df31de1820f6cf9d1c3462505f563e6b8
|
||||
// Generated with TypeScript: 6.0.3
|
||||
const __nodeRequire = require;
|
||||
@@ -729,7 +729,7 @@ function apiBindings(ast) {
|
||||
const failure = error
|
||||
? `(error) => { const status = error.status; const message = error.message; const data = error.data; ${error} }`
|
||||
: `(error) => { throw error; }`;
|
||||
return ` ${JSON.stringify(block.name)}: async (input) => context.callApi(${JSON.stringify(block.path)}, ${JSON.stringify(block.method)}, input).then((data) => { ${response} }).catch(${failure})`;
|
||||
return ` ${JSON.stringify(block.name)}: async (input) => context.callApi(${JSON.stringify(block.path)}, ${JSON.stringify(block.method)}, input).then((data) => { ${response} }, ${failure})`;
|
||||
});
|
||||
return members.length ? `const api = {\n${members.join(",\n")}\n };` : "";
|
||||
}
|
||||
@@ -1691,12 +1691,13 @@ async function __wrnexusResolveApiBinding(
|
||||
ctx: __WrnexusContext,
|
||||
): Promise<unknown> {
|
||||
if (binding.errorBody) {
|
||||
let data: unknown;
|
||||
try {
|
||||
const data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
} catch (err) {
|
||||
return __wrnexusEvalError(err, binding.errorBody, binding.helpers, ctx);
|
||||
}
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
}
|
||||
const data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// WRN editor extension source hash: bdf6f7f12c426181cad9a70f9ad1f3df2212bc487116ff3af97d2effd68b0c53
|
||||
// WRN editor extension source hash: 548e7e0c27d5c951325c977cc22f2ee0340dc3e34bd896904e519ee357ca37a8
|
||||
// WRN editor extension generator hash: 456d1d614e44e5fb1f19b784176c09cf2ade9b64ef73a17934c2698150b62728
|
||||
"use strict";
|
||||
var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports);
|
||||
|
||||
@@ -329,7 +329,7 @@ function apiBindings(ast: PageAst): string {
|
||||
|
||||
return ` ${JSON.stringify(block.name)}: async (input) => context.callApi(${JSON.stringify(
|
||||
block.path,
|
||||
)}, ${JSON.stringify(block.method)}, input).then((data) => { ${response} }).catch(${failure})`;
|
||||
)}, ${JSON.stringify(block.method)}, input).then((data) => { ${response} }, ${failure})`;
|
||||
});
|
||||
|
||||
return members.length ? `const api = {\n${members.join(",\n")}\n };` : "";
|
||||
|
||||
@@ -1056,12 +1056,13 @@ async function __wrnexusResolveApiBinding(
|
||||
ctx: __WrnexusContext,
|
||||
): Promise<unknown> {
|
||||
if (binding.errorBody) {
|
||||
let data: unknown;
|
||||
try {
|
||||
const data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
} catch (err) {
|
||||
return __wrnexusEvalError(err, binding.errorBody, binding.helpers, ctx);
|
||||
}
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
}
|
||||
const data = await __wrnexusCallApi(binding.path, binding.method, ctx);
|
||||
return __wrnexusEvalData(data, binding.body, binding.helpers, ctx);
|
||||
|
||||
@@ -1,7 +1,15 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { afterEach, expect, test } from "bun:test";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { parse } from "@wrnexus/syntax";
|
||||
import { generateTargets } from "../src/targets.ts";
|
||||
|
||||
const roots: string[] = [];
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function browserModule(inner: string): string {
|
||||
return generateTargets(
|
||||
parse(`page Repro {
|
||||
@@ -143,6 +151,111 @@ test("a page with state api and no client api blocks still reads that state (B5)
|
||||
}).not.toThrow();
|
||||
});
|
||||
|
||||
/**
|
||||
* Builds a browser module whose `run()` function calls api.searchUsers and
|
||||
* reports the outcome through `output.report(...)` so the test can observe
|
||||
* whether the call resolved or rejected without reaching into codegen
|
||||
* internals.
|
||||
*/
|
||||
function reportingBrowserModule(apiBlock: string): string {
|
||||
return generateTargets(
|
||||
parse(`page Repro {
|
||||
client {
|
||||
${apiBlock}
|
||||
}
|
||||
|
||||
outputs {
|
||||
report(payload: any)
|
||||
}
|
||||
|
||||
functions {
|
||||
client async function run(): Promise<void> {
|
||||
try {
|
||||
const users = await api.searchUsers({ name: "Ajay" })
|
||||
output.report({ ok: true, users })
|
||||
} catch (e) {
|
||||
output.report({ ok: false, message: String(e && e.message || e) })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
view { <main><button @click="run()">go</button></main> }
|
||||
}
|
||||
`),
|
||||
).browser;
|
||||
}
|
||||
|
||||
async function importBrowserModule(source: string): Promise<any> {
|
||||
const root = mkdtempSync(join(tmpdir(), "wrnexus-client-exec-"));
|
||||
roots.push(root);
|
||||
mkdirSync(root, { recursive: true });
|
||||
const file = join(root, "page.mjs");
|
||||
writeFileSync(file, source);
|
||||
return import(`${file.replace(/\\/g, "/")}?t=${Date.now()}`);
|
||||
}
|
||||
|
||||
test("a response body error is not swallowed by the error section (client)", async () => {
|
||||
const mod = await importBrowserModule(
|
||||
reportingBrowserModule(` api searchUsers GET /api/users {
|
||||
request { parameters { name: string } }
|
||||
response {
|
||||
return data.users.missing.length
|
||||
}
|
||||
error {
|
||||
return []
|
||||
}
|
||||
}`),
|
||||
);
|
||||
|
||||
const reports: unknown[] = [];
|
||||
const context = {
|
||||
state: {},
|
||||
props: {},
|
||||
output: { report: (value: unknown) => reports.push(value) },
|
||||
server: {},
|
||||
refs: {},
|
||||
callApi: async () => ({ users: [] }),
|
||||
};
|
||||
|
||||
await mod.__wrnexusClientFunctions.run(context);
|
||||
|
||||
expect(reports).toEqual([{ ok: false, message: expect.any(String) }]);
|
||||
// The error section's own fallback ("[]" / an empty array) must not have
|
||||
// been what the caller observed -- a bug in the response body is a
|
||||
// rejection, not a silently-returned fallback value.
|
||||
expect(reports[0]).not.toEqual({ ok: true, users: [] });
|
||||
});
|
||||
|
||||
test("a genuine transport failure still runs the error section's fallback (client)", async () => {
|
||||
const mod = await importBrowserModule(
|
||||
reportingBrowserModule(` api searchUsers GET /api/users {
|
||||
request { parameters { name: string } }
|
||||
response {
|
||||
return data.users
|
||||
}
|
||||
error {
|
||||
return ["fallback"]
|
||||
}
|
||||
}`),
|
||||
);
|
||||
|
||||
const reports: unknown[] = [];
|
||||
const context = {
|
||||
state: {},
|
||||
props: {},
|
||||
output: { report: (value: unknown) => reports.push(value) },
|
||||
server: {},
|
||||
refs: {},
|
||||
callApi: async () => {
|
||||
throw Object.assign(new Error("transport failed"), { status: 500 });
|
||||
},
|
||||
};
|
||||
|
||||
await mod.__wrnexusClientFunctions.run(context);
|
||||
|
||||
expect(reports).toEqual([{ ok: true, users: ["fallback"] }]);
|
||||
});
|
||||
|
||||
test("a state field named api does not collide with the emitted api object", () => {
|
||||
const generated = generateTargets(
|
||||
parse(`page Repro {
|
||||
|
||||
@@ -167,6 +167,83 @@ test("an ssr block used in {#each} with an error section runs the error body on
|
||||
expect(html).toContain("fallback");
|
||||
});
|
||||
|
||||
test("an ssr block's response body error is not swallowed by the error section", async () => {
|
||||
const generated = generate(
|
||||
parse(`page Repro {
|
||||
ssr {
|
||||
api ssrUsers GET /api/users {
|
||||
response {
|
||||
return data.users.missing.length
|
||||
}
|
||||
error {
|
||||
return ["fallback"]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
view { <main>{#each ssrUsers as u}<p>{u}</p>{/each}</main> }
|
||||
}
|
||||
`),
|
||||
);
|
||||
|
||||
const root = mkdtempSync(join(tmpdir(), "wrnexus-ssr-response-throws-"));
|
||||
roots.push(root);
|
||||
mkdirSync(root, { recursive: true });
|
||||
const file = join(root, "page.ts");
|
||||
writeFileSync(file, generated);
|
||||
|
||||
const mod = await import(`${file.replace(/\\/g, "/")}?t=${Date.now()}`);
|
||||
|
||||
await expect(
|
||||
mod.default({
|
||||
req: { url: "http://localhost/", headers: new Headers() },
|
||||
cookies: {},
|
||||
session: {},
|
||||
localStorage: {},
|
||||
__wrnexusCallApi: async () => ({ users: [] }),
|
||||
}),
|
||||
).rejects.toThrow();
|
||||
});
|
||||
|
||||
test("an ssr block still runs the error body on a genuine transport failure", async () => {
|
||||
const generated = generate(
|
||||
parse(`page Repro {
|
||||
ssr {
|
||||
api ssrUsers GET /api/users {
|
||||
response {
|
||||
return data.users.length
|
||||
}
|
||||
error {
|
||||
return ["fallback"]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
view { <main>{#each ssrUsers as u}<p>{u}</p>{/each}</main> }
|
||||
}
|
||||
`),
|
||||
);
|
||||
|
||||
const root = mkdtempSync(join(tmpdir(), "wrnexus-ssr-transport-fails-"));
|
||||
roots.push(root);
|
||||
mkdirSync(root, { recursive: true });
|
||||
const file = join(root, "page.ts");
|
||||
writeFileSync(file, generated);
|
||||
|
||||
const mod = await import(`${file.replace(/\\/g, "/")}?t=${Date.now()}`);
|
||||
const html = await mod.default({
|
||||
req: { url: "http://localhost/", headers: new Headers() },
|
||||
cookies: {},
|
||||
session: {},
|
||||
localStorage: {},
|
||||
__wrnexusCallApi: async () => {
|
||||
throw new Error("boom");
|
||||
},
|
||||
});
|
||||
|
||||
expect(html).toContain("fallback");
|
||||
});
|
||||
|
||||
test("an ssr block used in {#each} without an error section still propagates a failure", async () => {
|
||||
const generated = generate(
|
||||
parse(`page Repro {
|
||||
|
||||
@@ -98,6 +98,28 @@ test("with no second argument, a malformed or absent POST body falls back withou
|
||||
});
|
||||
});
|
||||
|
||||
// GET query strings travel as text (`URLSearchParams` values are always
|
||||
// strings), so a `v.number()` field must come back as a real number, not the
|
||||
// string the wire actually carried, or a page declaring `age?: number` on a
|
||||
// GET api block would be lying about the type. checkField in
|
||||
// @wrnexus/validation coerces via Number(pre) for both optional and required
|
||||
// number fields (see packages/validation/src/index.ts); this locks that in
|
||||
// end-to-end through defineEndpoint's own GET query-string resolution path.
|
||||
test("a GET request coerces a v.number() query param to an actual number", async () => {
|
||||
const ageSchema = v.object({ age: v.number() });
|
||||
const ageEndpoint = defineEndpoint<{ age: number }, { age: number; typeofAge: string }>({
|
||||
input: ageSchema,
|
||||
handler(input) {
|
||||
return { age: input.age, typeofAge: typeof input.age };
|
||||
},
|
||||
});
|
||||
const request = new Request("https://example.test/api/age?age=30");
|
||||
const ctx = createContext(request, new URL(request.url));
|
||||
const response = await ageEndpoint(ctx);
|
||||
expect(response.status).toBe(200);
|
||||
expect(await response.json()).toEqual({ data: { age: 30, typeofAge: "number" } });
|
||||
});
|
||||
|
||||
test("an explicit rawInput argument still wins and the request is never read", async () => {
|
||||
// A request whose body has already been consumed: if the endpoint tried to read it
|
||||
// again (rather than trusting the explicit rawInput), this would throw.
|
||||
|
||||
Reference in New Issue
Block a user