From f5294c581d97c51599a61c4e14994e77fea6130e Mon Sep 17 00:00:00 2001 From: Jeremy Wharton Date: Wed, 9 Sep 2026 00:23:38 -0500 Subject: [PATCH] Improve description of responses with the same status code An operation may have multiple responses with the same status code. Previously, only one response per status code was considered when determining the description for that status code. The resolution rules were as follows: 1. If the response was a member of a union, use the outermost union documentation comment if it exists. 2. If the response was an envelope, use the envelope's `@doc` if it exists. 3. If the response was an `@error` model, use the operation's `@errorsDoc` if it exists; otherwise, use the operation's `@returnsDoc` if it exists. 4. Otherwise, fall back to the default RFC 2616 description for the status code. Now, all responses sharing a status code are considered: 1. Compute the description each response would produce if it were the only response with that status code (its union comment, falling back to its envelope comment). 2. If every response sharing the status code produces the same description, then use that description. 3. Otherwise, if all such responses are `@error` models, use the operation's `@errorsDoc`; if none are, use the operation's `@returnsDoc`. 4. Otherwise, fall back to the default RFC 2616 description for the status code. --- ...ptions-same-status-code-2026-8-9-2-8-16.md | 20 ++ packages/http/src/responses.ts | 194 +++++++++++------- .../http/test/response-descriptions.test.ts | 174 ++++++++++++++++ .../test/response-descriptions.test.ts | 186 ++++++++++++++--- 4 files changed, 478 insertions(+), 96 deletions(-) create mode 100644 .chronus/changes/improve-response-descriptions-same-status-code-2026-8-9-2-8-16.md diff --git a/.chronus/changes/improve-response-descriptions-same-status-code-2026-8-9-2-8-16.md b/.chronus/changes/improve-response-descriptions-same-status-code-2026-8-9-2-8-16.md new file mode 100644 index 00000000000..f61f827e5d0 --- /dev/null +++ b/.chronus/changes/improve-response-descriptions-same-status-code-2026-8-9-2-8-16.md @@ -0,0 +1,20 @@ +--- +changeKind: fix +packages: + - "@typespec/http" + - "@typespec/openapi3" +--- + +Improve how response descriptions are chosen when multiple responses share a status code. When an operation has multiple responses with the same HTTP status code, all responses sharing that code are considered when computing the description associated with that code instead of just one. + +```tsp +@doc("A cat.") +model Cat { @statusCode _: 200, meow: boolean } + +@doc("A dog.") +model Dog { @statusCode _: 200, bark: boolean } + +// The response description defaults to "The request has succeeded." +// because the doc comments for the variants conflict. +op read(): Cat | Dog; +``` diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts index dfc2f920df7..ccfc0816683 100644 --- a/packages/http/src/responses.ts +++ b/packages/http/src/responses.ts @@ -26,7 +26,13 @@ import type { HttpProperty } from "./http-property.js"; import { HttpStateKeys, reportDiagnostic } from "./lib.js"; import { Visibility } from "./metadata.js"; import { HttpPayloadDisposition, resolveHttpPayload } from "./payload.js"; -import type { HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js"; +import type { + HttpOperationResponse, + HttpOperationResponseContent, + HttpPayloadBody, + HttpStatusCodes, + HttpStatusCodesEntry, +} from "./types.js"; /** * Get the responses for a given operation. @@ -36,16 +42,49 @@ export function getResponsesForOperation( operation: Operation, ): [HttpOperationResponse[], readonly Diagnostic[]] { const diagnostics = createDiagnosticCollector(); - const responses = new ResponseIndex(); // Resolve union variants into concrete response types, grouping plain body variants // (no HTTP metadata) into a single union type. const variants = resolveResponseVariants(program, operation.returnType); + const processedResponses: ProcessedResponseType[] = []; for (const { type, description } of variants) { - processResponseType(program, diagnostics, operation, responses, type, description); + processedResponses.push( + processResponseType(program, diagnostics, operation, type, description), + ); + } + + const responsesByStatus = new ResponseIndex(); + for (const response of processedResponses) { + for (const statusCode of response.statusCodes) { + responsesByStatus.add(statusCode, response); + } + } + + const responses: HttpOperationResponse[] = []; + for (const [statusCode, responseGroup] of responsesByStatus.entries()) { + const responseContents: HttpOperationResponseContent[] = responseGroup.map((response) => { + const content: HttpOperationResponseContent = { + headers: response.headers, + properties: response.properties, + }; + if (response.body) { + content.body = response.body; + } + return content; + }); + + responses.push({ + statusCodes: statusCode, + // It would be more accurate to express the response type as a union of all variant types. + // However, downstream code written since we first decided to use only the first variant's + // type may rely on us to continue doing so. For now, keep this behavior. + type: responseGroup[0].type, + description: getResponsesDescription(program, operation, statusCode, responseGroup), + responses: responseContents, + }); } - return diagnostics.wrap(responses.values()); + return diagnostics.wrap(responses); } interface ResolvedResponseVariant { @@ -110,37 +149,57 @@ function resolveResponseVariants( * Class keeping an index of all the response by status code */ class ResponseIndex { - readonly #index = new Map(); + readonly #index = new Map(); - public get(statusCode: HttpStatusCodesEntry): HttpOperationResponse | undefined { - return this.#index.get(this.#indexKey(statusCode)); - } - - public set(statusCode: HttpStatusCodesEntry, response: HttpOperationResponse): void { - this.#index.set(this.#indexKey(statusCode), response); + public add(statusCode: HttpStatusCodesEntry, response: ProcessedResponseType): void { + const indexKey = this.#indexKey(statusCode); + if (this.#index.has(indexKey)) { + this.#index.get(indexKey)!.push(response); + return; + } + this.#index.set(indexKey, [response]); } - public values(): HttpOperationResponse[] { - return [...this.#index.values()]; + public *entries(): MapIterator<[HttpStatusCodesEntry, ProcessedResponseType[]]> { + for (const [indexKey, responses] of this.#index.entries()) { + let parsedStatusCodes: HttpStatusCodesEntry; + if (indexKey === "*") { + parsedStatusCodes = "*"; + } else if (indexKey.includes(",")) { + const [start, end] = indexKey.split(","); + parsedStatusCodes = { start: Number(start), end: Number(end) }; + } else { + parsedStatusCodes = Number(indexKey); + } + yield [parsedStatusCodes, responses]; + } } #indexKey(statusCode: HttpStatusCodesEntry) { if (typeof statusCode === "number" || statusCode === "*") { return String(statusCode); } else { - return `${statusCode.start}-${statusCode.end}`; + return `${statusCode.start},${statusCode.end}`; } } } +interface ProcessedResponseType { + statusCodes: HttpStatusCodes; + type: Type; + parentDescription?: string; + body?: HttpPayloadBody; + headers: Record; + properties: HttpProperty[]; +} + function processResponseType( program: Program, diagnostics: DiagnosticCollector, operation: Operation, - responses: ResponseIndex, responseType: Type, - parentDescription?: string, -) { + parentDescription: string | undefined, +): ProcessedResponseType { // Get body const verb = getOperationVerb(program, operation); let { body: resolvedBody, metadata } = diagnostics.pipe( @@ -171,35 +230,14 @@ function processResponseType( } } - // Put them into currentEndpoint.responses - for (const statusCode of statusCodes) { - // the first model for this statusCode/content type pair carries the - // description for the endpoint. This could probably be improved. - const response: HttpOperationResponse = responses.get(statusCode) ?? { - statusCodes: statusCode, - type: responseType, - description: getResponseDescription( - program, - operation, - responseType, - statusCode, - metadata, - parentDescription, - ), - responses: [], - }; - - if (resolvedBody !== undefined) { - response.responses.push({ - body: resolvedBody, - headers, - properties: metadata, - }); - } else { - response.responses.push({ headers, properties: metadata }); - } - responses.set(statusCode, response); - } + return { + statusCodes: statusCodes, + type: responseType, + parentDescription, + body: resolvedBody, + headers: headers, + properties: metadata, + }; } /** @@ -290,39 +328,53 @@ function isPlainResponseBody(program: Program, type: Type): boolean { return !result || !result.metadata.some((p) => p.kind !== "bodyProperty"); } -function getResponseDescription( +function getResponsesDescription( program: Program, operation: Operation, - responseType: Type, statusCode: HttpStatusCodes[number], - metadata: HttpProperty[], - parentDescription?: string, -): string | undefined { - // If a parent union provided a description, use that first - if (parentDescription) { - return parentDescription; + variants: ProcessedResponseType[], +) { + if (variants.length <= 0) { + return getStatusCodeDescription(statusCode); } - // NOTE: If the response type is an envelope and not the same as the body - // type, then use its @doc as the response description. However, if the - // response type is the same as the body type, then use the default status - // code description and don't duplicate the schema description of the body - // as the response description. This allows more freedom to change how - // TypeSpec is expressed in semantically equivalent ways without causing - // the output to change unnecessarily. - if (isResponseEnvelope(metadata)) { - const desc = getDoc(program, responseType); - if (desc) { - return desc; + function getSingleResponseDescription(variant: ProcessedResponseType): string | undefined { + if (variant.parentDescription) { + return variant.parentDescription; + } + + // NOTE: If the response type includes response envelope metadata (e.g. @statusCode, @header), + // then use its @doc as the response description. Plain body types intentionally fall back to + // the status-code/operation-level descriptions to avoid duplicating the schema description as + // the response description. + if (isResponseEnvelope(variant.properties)) { + const desc = getDoc(program, variant.type); + if (desc) return desc; } + + return undefined; } - const desc = isErrorModel(program, responseType) - ? getErrorsDoc(program, operation) - : getReturnsDoc(program, operation); - if (desc) { - return desc; + const firstDesc = getSingleResponseDescription(variants[0]); + if (firstDesc && variants.every((v) => getSingleResponseDescription(v) === firstDesc)) { + return firstDesc; } - return getStatusCodeDescription(statusCode); + let hasError = false, + hasSuccess = false; + for (const variant of variants) { + if (isErrorModel(program, variant.type)) { + hasError = true; + } else { + hasSuccess = true; + } + } + + let desc: string | undefined; + if (hasSuccess && !hasError) { + desc = getReturnsDoc(program, operation); + } else if (hasError && !hasSuccess) { + desc = getErrorsDoc(program, operation); + } + return desc || getStatusCodeDescription(statusCode); } diff --git a/packages/http/test/response-descriptions.test.ts b/packages/http/test/response-descriptions.test.ts index 78acfd71252..1c44ff96380 100644 --- a/packages/http/test/response-descriptions.test.ts +++ b/packages/http/test/response-descriptions.test.ts @@ -79,3 +79,177 @@ it("@doc on response model set response doc if model is an evelope with @statusC ); strictEqual(op.responses[0].description, "Explicit doc"); }); + +it("uses union @doc when all responses sharing a status code came from that union", async () => { + const op = await getHttpOp(` + @doc("A cat or a dog.") + union Pet { cat: Cat, dog: Dog } + + model Cat { @statusCode _: 200, meow: boolean } + model Dog { @statusCode _: 200, bark: boolean } + + op read(): Pet; + `); + strictEqual(op.responses[0].description, "A cat or a dog."); +}); + +it("uses union @doc over operation @returnsDoc", async () => { + const op = await getHttpOp(` + @doc("A cat or a dog.") + union Pet { cat: Cat, dog: Dog } + + model Cat { @statusCode _: 200, meow: boolean } + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + op read(): Pet; + `); + strictEqual(op.responses[0].description, "A cat or a dog."); +}); + +it("uses shared @doc among all responses sharing a status code", async () => { + const op = await getHttpOp(` + @doc("A pet.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A pet.") + model Dog { @statusCode _: 200, bark: boolean } + + op read(): Cat | Dog; + `); + strictEqual(op.responses[0].description, "A pet."); +}); + +it("uses default description when the @doc of all responses sharing a status code disagree", async () => { + const op = await getHttpOp(` + @doc("A cat.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A dog.") + model Dog { @statusCode _: 200, bark: boolean } + + op read(): Cat | Dog; + `); + strictEqual(op.responses[0].description, "The request has succeeded."); +}); + +it("uses @returnsDoc when the @doc of all success responses sharing a status code disagree", async () => { + const op = await getHttpOp(` + @doc("A cat.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A dog.") + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + @errorsDoc("Something went wrong.") + op read(): Cat | Dog; + `); + strictEqual(op.responses[0].description, "A pet."); +}); + +it("uses @errorsDoc when the @doc of all @error responses sharing a status code disagree", async () => { + const op = await getHttpOp(` + @doc("Error A.") + @error model ErrorA { @statusCode _: 400, codeA: string } + + @doc("Error B.") + @error model ErrorB { @statusCode _: 400, codeB: string } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): ErrorA | ErrorB; + `); + strictEqual(op.responses[0].description, "Something went wrong."); +}); + +it("uses @returnsDoc when every response sharing a status code is a non-error model", async () => { + const op = await getHttpOp(` + union Pet { cat: Cat, dog: Dog } + + model Cat { @statusCode _: 200, meow: boolean } + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + @errorsDoc("Something went wrong.") + op read(): Pet; + `); + strictEqual(op.responses[0].description, "A pet."); +}); + +it("uses @errorsDoc when every response sharing a status code is an @error model", async () => { + const op = await getHttpOp(` + @error model ErrorA { @statusCode _: 400, codeA: string } + @error model ErrorB { @statusCode _: 400, codeB: string } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): ErrorA | ErrorB; + `); + strictEqual(op.responses[0].description, "Something went wrong."); +}); + +it("uses default description when responses sharing a status code mix success and error models", async () => { + const op = await getHttpOp(` + @error model Error { @statusCode _: 200; message: string } + model Pet { @statusCode _: 200 } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): Pet | Error; + `); + strictEqual(op.responses[0].description, "The request has succeeded."); +}); + +it("uses shared @doc for responses sharing a status code even when one comes from a union @doc and another from its own @doc", async () => { + const op = await getHttpOp(` + @doc("Success.") + union Pet { cat: Cat } + + model Cat { @statusCode _: 200, meow: boolean } + + @doc("Success.") + model Extra { @statusCode _: 200, extra: string } + + op read(): Pet | Extra; + `); + strictEqual(op.responses[0].description, "Success."); +}); + +it("uses shared @doc among all responses sharing a status code range", async () => { + const op = await getHttpOp(` + @doc("A pet.") + model Cat { + @statusCode @minValue(200) @maxValue(299) _: int32; + meow: boolean; + } + + @doc("A pet.") + model Dog { + @statusCode @minValue(200) @maxValue(299) _: int32; + bark: boolean; + } + + op read(): Cat | Dog; + `); + strictEqual(op.responses[0].description, "A pet."); +}); + +it("uses default description when responses sharing a status code range have different descriptions", async () => { + const op = await getHttpOp(` + @doc("A cat.") + model Cat { + @statusCode @minValue(200) @maxValue(299) _: int32; + meow: boolean; + } + + @doc("A dog.") + model Dog { + @statusCode @minValue(200) @maxValue(299) _: int32; + bark: boolean; + } + + op read(): Cat | Dog; + `); + strictEqual(op.responses[0].description, "Successful"); +}); diff --git a/packages/openapi3/test/response-descriptions.test.ts b/packages/openapi3/test/response-descriptions.test.ts index bbfa05180a2..a8c5aaf4615 100644 --- a/packages/openapi3/test/response-descriptions.test.ts +++ b/packages/openapi3/test/response-descriptions.test.ts @@ -62,31 +62,6 @@ worksFor(supportedVersions, ({ openApiFor }) => { strictEqual(res.paths["/"].get.responses["default"].description, "Generic error"); }); - it("uses first model's description when multiple models have same status code", async () => { - const res = await openApiFor( - ` - @doc("Foo") model Foo { @statusCode _: 409 } - @doc("Bar") model Bar { @statusCode _: 409 } - op read(): { @statusCode _: 200, content: string } | Foo | Bar; - `, - ); - strictEqual(res.paths["/"].get.responses["200"].description, "The request has succeeded."); - strictEqual(res.paths["/"].get.responses["409"].description, "Foo"); - }); - - it("expands named union in return type and uses first variant's description", async () => { - const res = await openApiFor( - ` - @doc("Foo") model Foo { @statusCode _: 409 } - @doc("Bar") model Bar { @statusCode _: 409 } - union Conflict { Foo: Foo; Bar: Bar }; - op read(): { @statusCode _: 200, content: string } | Conflict; - `, - ); - strictEqual(res.paths["/"].get.responses["200"].description, "The request has succeeded."); - strictEqual(res.paths["/"].get.responses["409"].description, "Foo"); - }); - it("uses union variant descriptions", async () => { const res = await openApiFor( ` @@ -155,4 +130,165 @@ worksFor(supportedVersions, ({ openApiFor }) => { strictEqual(res.paths["/"].get.responses["401"].description, "Inner authentication errors"); strictEqual(res.paths["/"].get.responses["403"].description, "All error responses"); }); + + it("uses union @doc over operation @returnsDoc", async () => { + const res = await openApiFor(` + @doc("A cat or a dog.") + union Pet { cat: Cat, dog: Dog } + + model Cat { @statusCode _: 200, meow: boolean } + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + op read(): Pet; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "A cat or a dog."); + }); + + it("uses shared @doc among all responses sharing a status code", async () => { + const res = await openApiFor(` + @doc("A pet.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A pet.") + model Dog { @statusCode _: 200, bark: boolean } + + op read(): Cat | Dog; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "A pet."); + }); + + it("uses default description when the @doc of all responses sharing a status code disagree", async () => { + const res = await openApiFor(` + @doc("A cat.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A dog.") + model Dog { @statusCode _: 200, bark: boolean } + + op read(): Cat | Dog; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "The request has succeeded."); + }); + + it("uses @returnsDoc when the @doc of all success responses sharing a status code disagree", async () => { + const res = await openApiFor(` + @doc("A cat.") + model Cat { @statusCode _: 200, meow: boolean } + + @doc("A dog.") + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + @errorsDoc("Something went wrong.") + op read(): Cat | Dog; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "A pet."); + }); + + it("uses @errorsDoc when the @doc of all @error responses sharing a status code disagree", async () => { + const res = await openApiFor(` + @doc("Error A.") + @error model ErrorA { @statusCode _: 400, codeA: string } + + @doc("Error B.") + @error model ErrorB { @statusCode _: 400, codeB: string } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): ErrorA | ErrorB; + `); + strictEqual(res.paths["/"].get.responses["400"].description, "Something went wrong."); + }); + + it("uses @returnsDoc when every response sharing a status code is a non-error model", async () => { + const res = await openApiFor(` + union Pet { cat: Cat, dog: Dog } + + model Cat { @statusCode _: 200, meow: boolean } + model Dog { @statusCode _: 200, bark: boolean } + + @returnsDoc("A pet.") + @errorsDoc("Something went wrong.") + op read(): Pet; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "A pet."); + }); + + it("uses @errorsDoc when every response sharing a status code is an @error model", async () => { + const res = await openApiFor(` + @error model ErrorA { @statusCode _: 400, codeA: string } + @error model ErrorB { @statusCode _: 400, codeB: string } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): ErrorA | ErrorB; + `); + strictEqual(res.paths["/"].get.responses["400"].description, "Something went wrong."); + }); + + it("uses default description when responses sharing a status code mix success and error models", async () => { + const res = await openApiFor(` + @error model Error { @statusCode _: 200; message: string } + model Pet { @statusCode _: 200 } + + @returnsDoc("Success.") + @errorsDoc("Something went wrong.") + op read(): Pet | Error; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "The request has succeeded."); + }); + + it("uses shared @doc for responses sharing a status code even when one comes from a union @doc and another from its own @doc", async () => { + const res = await openApiFor(` + @doc("Success.") + union Pet { cat: Cat } + + model Cat { @statusCode _: 200, meow: boolean } + + @doc("Success.") + model Extra { @statusCode _: 200, extra: string } + + op read(): Pet | Extra; + `); + strictEqual(res.paths["/"].get.responses["200"].description, "Success."); + }); + + it("uses shared @doc among all responses sharing a status code range", async () => { + const res = await openApiFor(` + @doc("A pet.") + model Cat { + @statusCode @minValue(200) @maxValue(299) _: int32; + meow: boolean; + } + + @doc("A pet.") + model Dog { + @statusCode @minValue(200) @maxValue(299) _: int32; + bark: boolean; + } + + op read(): Cat | Dog; + `); + strictEqual(res.paths["/"].get.responses["2XX"].description, "A pet."); + }); + + it("uses default description when responses sharing a status code range have different descriptions", async () => { + const res = await openApiFor(` + @doc("A cat.") + model Cat { + @statusCode @minValue(200) @maxValue(299) _: int32; + meow: boolean; + } + + @doc("A dog.") + model Dog { + @statusCode @minValue(200) @maxValue(299) _: int32; + bark: boolean; + } + + op read(): Cat | Dog; + `); + strictEqual(res.paths["/"].get.responses["2XX"].description, "Successful"); + }); });