Merge pull request #5 from Tam2/fix/no-body-command-schema

fix: handle no-body / optional-body endpoints and empty 2xx responses

Author: Tam2

src/generator.ts

@@ -54,12 +54,58 @@ function extractPathBlock(content: string, startIndex: number): string | null {
   return content.slice(startIndex, pos);
 }
 
+/**
+ * Extract the `{...}` block for a named member of an interface, e.g. an entry of `operations`.
+ * Handles both bare identifier keys (`updatePet: {`) and quoted keys (`"activity/star-repo": {`) —
+ * openapi-typescript quotes operationIds that contain non-identifier characters (`/`, `-`, `.`).
+ */
+function extractNamedBlock(content: string, name: string): string | null {
+  const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const re = new RegExp(`(?:^|[\\s{;,])["']?${escaped}["']?\\s*:\\s*\\{`, 'm');
+  const m = re.exec(content);
+  if (!m) return null;
+  const braceIdx = content.indexOf('{', m.index);
+  let depth = 1;
+  let pos = braceIdx + 1;
+  while (pos < content.length && depth > 0) {
+    if (content[pos] === '{') depth++;
+    else if (content[pos] === '}') depth--;
+    pos++;
+  }
+  return content.slice(braceIdx, pos);
+}
+
+/** A block has a request body if `requestBody` maps to an object; `requestBody?: never`/absent => no body. */
+function blockHasRequestBody(block: string): boolean {
+  return /requestBody\s*\??\s*:\s*\{/.test(block);
+}
+
+/**
+ * Whether an operation actually declares a request body. Resolves a `<method>: operations["Name"]`
+ * reference against the `operations` interface, or reads an inline `<method>: { ... }` block.
+ * Defaults to `true` when the operation can't be resolved, so a real body is never dropped.
+ */
+function operationHasBody(pathBlock: string, method: string, operationsContent: string): boolean {
+  const ref = new RegExp(`\\b${method}\\s*:\\s*operations\\[["']([^"']+)["']\\]`).exec(pathBlock);
+  if (ref) {
+    const opBlock = extractNamedBlock(operationsContent, ref[1]);
+    return opBlock ? blockHasRequestBody(opBlock) : true;
+  }
+  const inline = new RegExp(`\\b${method}\\s*:\\s*\\{`).exec(pathBlock);
+  if (inline) {
+    const inlineBlock = extractPathBlock(pathBlock, inline.index);
+    return inlineBlock ? blockHasRequestBody(inlineBlock) : true;
+  }
+  return true;
+}
+
 export function extractPaths(apiTypesContent: string): PathInfo[] {
   const pathsContent = extractInterfaceBody(apiTypesContent, 'paths');
   if (!pathsContent) {
     throw new Error('Could not find paths interface in api.d.ts');
   }
 
+  const operationsContent = extractInterfaceBody(apiTypesContent, 'operations') ?? '';
   const pathKeyRegex = /"(\/[^"]*)":\s*\{/g;
   const paths: PathInfo[] = [];
 
@@ -84,7 +130,10 @@ export function extractPaths(apiTypesContent: string): PathInfo[] {
     for (const { method, regex } of methodChecks) {
       if (regex.test(pathBlock)) {
         const hasParams = pathStr.includes('{');
-        const hasBody = method !== 'get' && method !== 'delete';
+        const hasBody =
+          method !== 'get' &&
+          method !== 'delete' &&
+          operationHasBody(pathBlock, method, operationsContent);
         const hasQuery = method === 'get';
         methods.push({ method, hasParams, hasBody, hasQuery });
       }
@@ -177,7 +226,7 @@ export function generateFileContent(paths: PathInfo[], clientImport: string): st
         allHandlers.add(`handle${Method}Command`);
         allHandlers.add(`handle${Method}Form`);
         if (methodInfo.hasParams) allTypes.add('GetParameters');
-        allTypes.add('GetRequestBody');
+        if (methodInfo.hasBody) allTypes.add('GetRequestBody');
       }
     }
   }
@@ -226,22 +275,30 @@ function generateFunctionCode(
     const formName = pathToFunctionName(pathStr, method, 'Form');
     codes.push(`export const ${commandName} = command(\n\tz.custom<GetParameters<paths, '${pathStr}', 'delete'>>(),\n\tasync (params) => handleDeleteCommand('${pathStr}', params)\n);`);
     codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<GetParameters<paths, '${pathStr}', 'delete'>>()),\n\tasync (params) => handleDeleteForm('${pathStr}', params)\n);`);
-  } else if (info.hasParams) {
-    const commandName = pathToFunctionName(pathStr, method, 'Command');
-    const formName = pathToFunctionName(pathStr, method, 'Form');
-    const Method = method.charAt(0).toUpperCase() + method.slice(1);
-    const commandHandler = `handle${Method}Command`;
-    const formHandler = `handle${Method}Form`;
-    codes.push(`export const ${commandName} = command(\n\tz.object({\n\t\tpath: z.custom<GetParameters<paths, '${pathStr}', '${method}'>['path']>(),\n\t\tbody: z.custom<GetRequestBody<paths, '${pathStr}', '${method}'>>()\n\t}),\n\tasync (input) => ${commandHandler}('${pathStr}', input)\n);`);
-    codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<{ path: GetParameters<paths, '${pathStr}', '${method}'>['path']; body: GetRequestBody<paths, '${pathStr}', '${method}'> }>()),\n\tasync (input) => ${formHandler}('${pathStr}', input)\n);`);
   } else {
     const commandName = pathToFunctionName(pathStr, method, 'Command');
     const formName = pathToFunctionName(pathStr, method, 'Form');
     const Method = method.charAt(0).toUpperCase() + method.slice(1);
     const commandHandler = `handle${Method}Command`;
     const formHandler = `handle${Method}Form`;
-    codes.push(`export const ${commandName} = command(\n\tz.custom<GetRequestBody<paths, '${pathStr}', '${method}'>>(),\n\tasync (body) => ${commandHandler}('${pathStr}', body)\n);`);
-    codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<GetRequestBody<paths, '${pathStr}', '${method}'>>()),\n\tasync (body) => ${formHandler}('${pathStr}', body)\n);`);
+    const pathType = `GetParameters<paths, '${pathStr}', '${method}'>['path']`;
+    const bodyType = `GetRequestBody<paths, '${pathStr}', '${method}'>`;
+
+    if (info.hasParams && info.hasBody) {
+      codes.push(`export const ${commandName} = command(\n\tz.object({\n\t\tpath: z.custom<${pathType}>(),\n\t\tbody: z.custom<${bodyType}>()\n\t}),\n\tasync (input) => ${commandHandler}('${pathStr}', input)\n);`);
+      codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<{ path: ${pathType}; body: ${bodyType} }>()),\n\tasync (input) => ${formHandler}('${pathStr}', input)\n);`);
+    } else if (info.hasParams && !info.hasBody) {
+      // Path params but no request body: only accept `{ path }` so callers don't need `body: {}`.
+      codes.push(`export const ${commandName} = command(\n\tz.object({\n\t\tpath: z.custom<${pathType}>()\n\t}),\n\tasync (input) => ${commandHandler}('${pathStr}', input)\n);`);
+      codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<{ path: ${pathType} }>()),\n\tasync (input) => ${formHandler}('${pathStr}', input)\n);`);
+    } else if (!info.hasParams && info.hasBody) {
+      codes.push(`export const ${commandName} = command(\n\tz.custom<${bodyType}>(),\n\tasync (body) => ${commandHandler}('${pathStr}', body)\n);`);
+      codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()).pipe(z.custom<${bodyType}>()),\n\tasync (body) => ${formHandler}('${pathStr}', body)\n);`);
+    } else {
+      // No path params and no request body: a no-argument action command (e.g. billing reactivate).
+      codes.push(`export const ${commandName} = command(\n\tz.void(),\n\tasync () => ${commandHandler}('${pathStr}')\n);`);
+      codes.push(`export const ${formName} = form(\n\tz.record(z.string(), z.any()),\n\tasync () => ${formHandler}('${pathStr}')\n);`);
+    }
   }
 
   return codes;

src/runtime/index.ts

@@ -13,54 +13,66 @@ function handleResponse(response: any, error: any, data: any) {
     throw svelteError(503, 'Network error: no response from service');
   }
   if (error) {
+    // Use the real HTTP status from the response (works for any API); fall back to 500 only if it's
+    // somehow absent. The error body's `message`, when present, gives a better message than a generic.
     throw svelteError(
-      (error as any).statusCode ?? 500,
-      (error as any).message ?? 'An error occurred',
+      response.status ?? 500,
+      (error as { message?: string })?.message ?? 'An error occurred',
     );
   }
-  if (!data) {
-    throw svelteError(404, 'Not found');
+  if (data === undefined || data === null) {
+    // A successful (2xx) response may legitimately carry no body — 204 No Content, or a void /
+    // action endpoint (e.g. "resend invitation"). Only treat missing data as an error when the
+    // response itself was not ok; otherwise return it so the caller sees success, not a false 404.
+    if (response.ok) return data;
+    throw svelteError(response.status ?? 404, 'Not found');
   }
   return data;
 }
 
+/**
+ * Build the openapi-fetch request options from a command's single input argument.
+ *
+ * The generated command wrappers pass their input in one of these shapes, and this normalizes them:
+ *  - `{ path, body }`          → params.path + body            (path + body endpoint)
+ *  - `{ body }`                → body                          (no-path endpoint, wrapped)
+ *  - `{ path }` (no `body`)    → params.path, NO body          (path-only endpoint, no request body)
+ *  - a bare body `{ ...fields }` → body                        (no-path endpoint whose wrapper passes
+ *                                                                the body directly)
+ *  - `undefined`               → neither                       (no-path, no-body action endpoint)
+ *
+ * The key subtlety: a bare body has no `body` key, so it must be forwarded whole; but a path-only
+ * input (has `path`, no `body`) must NOT be forwarded as the body, or `{ path }` would be sent as
+ * the request body to a no-body endpoint.
+ */
+function buildRequestOptions(input: any) {
+  const hasPath = input && typeof input === 'object' && 'path' in input;
+  const hasBody = input && typeof input === 'object' && 'body' in input;
+  const body = hasBody ? input.body : hasPath ? undefined : input;
+  return {
+    ...(hasPath && { params: { path: input.path } }),
+    ...(body !== undefined && { body }),
+  };
+}
+
 export function createRemoteHandlers(client: OpenapiClient) {
   async function handleGetQuery(path: string, params: any) {
     const { data, error, response } = await client.GET(path, { params });
     return handleResponse(response, error, data);
   }
 
   async function handlePostCommand(path: string, input: any) {
-    const hasPath = input && typeof input === 'object' && 'path' in input;
-    const hasBody = input && typeof input === 'object' && 'body' in input;
-    const { data, error, response } = await client.POST(path, {
-      ...(hasPath && { params: { path: input.path } }),
-      body: hasBody ? input.body : input,
-    });
+    const { data, error, response } = await client.POST(path, buildRequestOptions(input));
     return handleResponse(response, error, data);
   }
 
   async function handlePatchCommand(path: string, input: any) {
-    const hasPath = input && typeof input === 'object' && 'path' in input;
-    const hasBody = input && typeof input === 'object' && 'body' in input;
-    // For a no-path endpoint the generated wrapper passes the body directly (no `body` key), so fall
-    // back to the whole input as the body — matching handlePostCommand. Without this, a bare body was
-    // silently dropped, sending an empty PATCH.
-    const { data, error, response } = await client.PATCH(path, {
-      ...(hasPath && { params: { path: input.path } }),
-      body: hasBody ? input.body : input,
-    });
+    const { data, error, response } = await client.PATCH(path, buildRequestOptions(input));
     return handleResponse(response, error, data);
   }
 
   async function handlePutCommand(path: string, input: any) {
-    const hasPath = input && typeof input === 'object' && 'path' in input;
-    const hasBody = input && typeof input === 'object' && 'body' in input;
-    // Same bare-body fallback as handlePostCommand/handlePatchCommand.
-    const { data, error, response } = await client.PUT(path, {
-      ...(hasPath && { params: { path: input.path } }),
-      body: hasBody ? input.body : input,
-    });
+    const { data, error, response } = await client.PUT(path, buildRequestOptions(input));
     return handleResponse(response, error, data);
   }
 

src/types.ts

@@ -13,13 +13,20 @@ export type GetParameters<
   TMethod extends keyof Paths[TPath],
 > = Paths[TPath][TMethod] extends { parameters: infer P } ? P : never;
 
-/** Extract JSON request body for a given path and method */
+/**
+ * Extract the JSON request body for a given path and method.
+ *
+ * Uses `requestBody?:` (optional) in the constraint so it matches BOTH a required body
+ * (`requestBody: {...}`) and an optional one (`requestBody?: {...}`). openapi-typescript emits the
+ * optional form whenever the OpenAPI operation marks its body `required: false` — which is the
+ * common case — so matching only the required form typed those endpoints as `never`.
+ */
 export type GetRequestBody<
   Paths,
   TPath extends keyof Paths,
   TMethod extends keyof Paths[TPath],
 > = Paths[TPath][TMethod] extends {
-  requestBody: { content: { 'application/json': infer B } };
+  requestBody?: { content: { 'application/json': infer B } };
 }
   ? B
   : never;