-
Notifications
You must be signed in to change notification settings - Fork 0
Add HTTP status code descriptions and enhance response handling in Op… #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,34 +103,112 @@ function buildRequestBody( | |
| return body; | ||
| } | ||
|
|
||
| // Default descriptions for common HTTP status codes | ||
| const HTTP_STATUS_DESCRIPTIONS: Record<number, string> = { | ||
| 200: 'OK', | ||
| 201: 'Created', | ||
| 202: 'Accepted', | ||
| 204: 'No Content', | ||
| 400: 'Bad Request', | ||
| 401: 'Unauthorized', | ||
| 403: 'Forbidden', | ||
| 404: 'Not Found', | ||
| 409: 'Conflict', | ||
| 422: 'Unprocessable Entity', | ||
| 500: 'Internal Server Error' | ||
| }; | ||
|
|
||
| // Minimal inline schema for ProblemDetails error responses | ||
| const PROBLEM_DETAILS_SCHEMA = { | ||
| type: 'object', | ||
| properties: { | ||
| status: { type: 'integer' }, | ||
| title: { type: 'string' }, | ||
| detail: { type: 'string' } | ||
| } | ||
| }; | ||
|
|
||
| function buildResponses( | ||
| responseSchema: SchemaBuilder<any, any, any, any, any> | null, | ||
| meta: EndpointMetadata, | ||
| method: string | ||
| ): Record<string, unknown> { | ||
| if (responseSchema) { | ||
| const jsonSchema = convertSchema(responseSchema); | ||
| const respInfo = responseSchema.introspect() as any; | ||
| const result: Record<string, unknown> = {}; | ||
|
|
||
| // Multi-code path — .responses() was called | ||
| if (meta.responsesSchemas) { | ||
| for (const [codeStr, schema] of Object.entries(meta.responsesSchemas)) { | ||
| const code = Number(codeStr); | ||
| const desc = | ||
| HTTP_STATUS_DESCRIPTIONS[code] ?? `Response ${codeStr}`; | ||
| if (schema) { | ||
| const jsonSchema = convertSchema(schema); | ||
| const respInfo = schema.introspect() as any; | ||
| const customDesc = | ||
| typeof respInfo.description === 'string' && | ||
| respInfo.description !== '' | ||
| ? respInfo.description | ||
| : desc; | ||
| result[codeStr] = { | ||
| description: customDesc, | ||
| content: { 'application/json': { schema: jsonSchema } } | ||
| }; | ||
| } else { | ||
| result[codeStr] = { description: desc }; | ||
| } | ||
| } | ||
| } else if (meta.responseSchema) { | ||
|
Comment on lines
+137
to
+159
|
||
| // Legacy single-code path — .returns() was called | ||
| const jsonSchema = convertSchema(meta.responseSchema); | ||
| const respInfo = meta.responseSchema.introspect() as any; | ||
| const desc = | ||
| typeof respInfo.description === 'string' && | ||
| respInfo.description !== '' | ||
| ? respInfo.description | ||
| : 'Successful response'; | ||
| return { | ||
| '200': { | ||
| description: desc, | ||
| content: { | ||
| 'application/json': { schema: jsonSchema } | ||
| } | ||
| result['200'] = { | ||
| description: desc, | ||
| content: { 'application/json': { schema: jsonSchema } } | ||
| }; | ||
| } else if (method === 'DELETE' || method === 'HEAD') { | ||
| result['204'] = { description: 'No content' }; | ||
| } else { | ||
| result['200'] = { description: 'Successful response' }; | ||
| } | ||
|
|
||
| // Auto-add framework-generated error responses | ||
| if (meta.bodySchema && !result['422']) { | ||
| result['422'] = { | ||
| description: 'Validation error', | ||
| content: { | ||
| 'application/problem+json': { schema: PROBLEM_DETAILS_SCHEMA } | ||
| } | ||
| }; | ||
| } | ||
|
Comment on lines
+178
to
186
|
||
|
|
||
| // No response schema — use 204 for methods that typically don't return content | ||
| if (method === 'DELETE' || method === 'HEAD') { | ||
| return { '204': { description: 'No content' } }; | ||
| if (meta.authRoles !== null) { | ||
| if (!result['401']) { | ||
| result['401'] = { | ||
| description: 'Unauthorized', | ||
| content: { | ||
| 'application/problem+json': { | ||
| schema: PROBLEM_DETAILS_SCHEMA | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| if (!result['403']) { | ||
| result['403'] = { | ||
| description: 'Forbidden', | ||
| content: { | ||
| 'application/problem+json': { | ||
| schema: PROBLEM_DETAILS_SCHEMA | ||
| } | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return { '200': { description: 'Successful response' } }; | ||
| return result; | ||
| } | ||
|
|
||
| function buildOperation( | ||
|
|
@@ -217,10 +295,7 @@ function buildOperation( | |
| } | ||
|
|
||
| // Responses | ||
| operation['responses'] = buildResponses( | ||
| meta.responseSchema, | ||
| meta.method.toUpperCase() | ||
| ); | ||
| operation['responses'] = buildResponses(meta, meta.method.toUpperCase()); | ||
|
|
||
| // Security | ||
| const security = mapOperationSecurity(authRoles(meta), securitySchemeNames); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,35 +37,96 @@ export abstract class ActionResult { | |
| // ----------------------------------------------------------------------- | ||
|
|
||
| /** 200 OK — serializes value using content negotiation. */ | ||
| static ok(body: unknown, headers?: Record<string, string>): JsonResult { | ||
| return new JsonResult(body, 200, headers); | ||
| static ok<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<200, T> { | ||
| return new JsonResult(body, 200, headers) as JsonResult<200, T>; | ||
| } | ||
|
|
||
| /** 201 Created — serializes value using content negotiation. */ | ||
| static created( | ||
| body: unknown, | ||
| static created<T>( | ||
| body: T, | ||
| location?: string, | ||
| headers?: Record<string, string> | ||
| ): JsonResult { | ||
| ): JsonResult<201, T> { | ||
| const h: Record<string, string> = { ...headers }; | ||
| if (location) h['location'] = location; | ||
| return new JsonResult(body, 201, h); | ||
| return new JsonResult(body, 201, h) as JsonResult<201, T>; | ||
| } | ||
|
|
||
| /** 202 Accepted — serializes value using content negotiation. */ | ||
| static accepted<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<202, T> { | ||
| return new JsonResult(body, 202, headers) as JsonResult<202, T>; | ||
| } | ||
|
Comment on lines
39
to
64
|
||
|
|
||
| /** 204 No Content. */ | ||
| static noContent(): NoContentResult { | ||
| return new NoContentResult(); | ||
| } | ||
|
|
||
| /** 400 Bad Request — serializes value as JSON. */ | ||
| static badRequest<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<400, T> { | ||
| return new JsonResult(body, 400, headers) as JsonResult<400, T>; | ||
| } | ||
|
|
||
| /** 401 Unauthorized — serializes value as JSON. */ | ||
| static unauthorized<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<401, T> { | ||
| return new JsonResult(body, 401, headers) as JsonResult<401, T>; | ||
| } | ||
|
|
||
| /** 403 Forbidden — serializes value as JSON. */ | ||
| static forbidden<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<403, T> { | ||
| return new JsonResult(body, 403, headers) as JsonResult<403, T>; | ||
| } | ||
|
|
||
| /** 404 Not Found — serializes value as JSON. */ | ||
| static notFound<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<404, T> { | ||
| return new JsonResult(body, 404, headers) as JsonResult<404, T>; | ||
| } | ||
|
|
||
| /** 409 Conflict — serializes value as JSON. */ | ||
| static conflict<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<409, T> { | ||
| return new JsonResult(body, 409, headers) as JsonResult<409, T>; | ||
| } | ||
|
|
||
| /** Temporary (302) or permanent (301) redirect. */ | ||
| static redirect(url: string, permanent = false): RedirectResult { | ||
| return new RedirectResult(url, permanent); | ||
| } | ||
|
|
||
| /** Explicit JSON response — always uses application/json regardless of Accept. */ | ||
| /** | ||
| * Explicit JSON response with a specific status code. | ||
| * Use the named factories (`ok`, `notFound`, etc.) for common codes. | ||
| * This overload is an escape hatch for uncommon status codes. | ||
| */ | ||
| static json<T>(body: T): JsonResult<200, T>; | ||
| static json<S extends number, T>( | ||
| body: T, | ||
| status: S, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<S, T>; | ||
| static json( | ||
| body: unknown, | ||
| status = 200, | ||
| status: number = 200, | ||
| headers?: Record<string, string> | ||
| ): JsonResult { | ||
| return new JsonResult(body, status, headers); | ||
|
|
@@ -99,11 +160,11 @@ export abstract class ActionResult { | |
| } | ||
|
|
||
| /** Bare status code with no body. */ | ||
| static status( | ||
| status: number, | ||
| static status<S extends number>( | ||
| status: S, | ||
| headers?: Record<string, string> | ||
| ): StatusCodeResult { | ||
| return new StatusCodeResult(status, headers); | ||
| ): StatusCodeResult<S> { | ||
| return new StatusCodeResult(status, headers) as StatusCodeResult<S>; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -119,15 +180,22 @@ export abstract class ActionResult { | |
| * `ActionResult.ok()` and `ActionResult.created()` produce a {@link JsonResult} | ||
| * that goes through content negotiation instead. | ||
| */ | ||
| export class JsonResult extends ActionResult { | ||
| readonly body: unknown; | ||
| readonly status: number; | ||
| export class JsonResult< | ||
| TStatus extends number = number, | ||
| TBody = unknown | ||
| > extends ActionResult { | ||
| readonly body: TBody; | ||
| readonly status: TStatus; | ||
| readonly headers: Record<string, string>; | ||
|
|
||
| constructor(body: unknown, status = 200, headers?: Record<string, string>) { | ||
| constructor( | ||
| body: TBody, | ||
| status: TStatus | number = 200, | ||
| headers?: Record<string, string> | ||
| ) { | ||
| super(); | ||
| this.body = body; | ||
| this.status = status; | ||
| this.status = status as TStatus; | ||
| this.headers = headers ?? {}; | ||
| } | ||
|
|
||
|
|
@@ -270,13 +338,15 @@ export class StreamResult extends ActionResult { | |
| * Responds with a bare HTTP status code and no body. | ||
| * Created by `ActionResult.status()`. | ||
| */ | ||
| export class StatusCodeResult extends ActionResult { | ||
| readonly status: number; | ||
| export class StatusCodeResult< | ||
| TStatus extends number = number | ||
| > extends ActionResult { | ||
| readonly status: TStatus; | ||
| readonly headers: Record<string, string>; | ||
|
|
||
| constructor(status: number, headers?: Record<string, string>) { | ||
| constructor(status: TStatus | number, headers?: Record<string, string>) { | ||
| super(); | ||
| this.status = status; | ||
| this.status = status as TStatus; | ||
| this.headers = headers ?? {}; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PROBLEM_DETAILS_SCHEMAdoesn’t match the server’s actual RFC 9457 Problem Details payload (ProblemDetailsincludes a requiredtypefield and may includeinstanceplus extension members likeerrors). Consider including at leasttypeand allowing additional properties (and/or reusing theProblemDetailsshape from@cleverbrush/server) so the generated OpenAPI schema matches real responses.