-
Notifications
You must be signed in to change notification settings - Fork 239
Fix OpenAPI error examples missing when service-level and operation-level errors share HTTP status codes #2754
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
Conversation
71cb7a7
to
f0fda09
Compare
f0fda09
to
3f376e0
Compare
.../src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AbstractRestProtocol.java
Show resolved
Hide resolved
...i/src/test/java/software/amazon/smithy/openapi/fromsmithy/ServiceLevelErrorOverrideTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/software/amazon/smithy/openapi/fromsmithy/ErrorDeconflictingExamplesTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/software/amazon/smithy/openapi/fromsmithy/ErrorDeconflictingExamplesTest.java
Outdated
Show resolved
Hide resolved
// The bug: When ValidationException (service-level) and CustomError (operation-level) | ||
// both map to 400, the service-level error takes precedence and operation-specific | ||
// examples are lost | ||
assertTrue(jsonContent.getMember("examples").isPresent(), | ||
"BUG: Service-level ValidationException overrides operation-specific CustomError examples. " + | ||
"Expected CustomError examples to appear in 400 response but they are missing."); | ||
|
||
ObjectNode examples = jsonContent.expectMember("examples").expectObjectNode(); | ||
assertFalse(examples.getMembers().isEmpty(), | ||
"Expected CustomError examples in 400 response content"); | ||
|
||
// Look for our specific custom error example content | ||
boolean foundCustomErrorExample = examples.getMembers().values().stream() | ||
.filter(Node::isObjectNode) | ||
.map(node -> node.expectObjectNode()) | ||
.filter(example -> example.getMember("value").isPresent()) | ||
.map(example -> example.expectMember("value").expectObjectNode()) | ||
.anyMatch(value -> | ||
value.getMember("code").filter(node -> "CUSTOM_ERROR".equals(node.expectStringNode().getValue())).isPresent() && | ||
value.getMember("message").filter(node -> "Custom error occurred".equals(node.expectStringNode().getValue())).isPresent() | ||
); | ||
|
||
assertTrue(foundCustomErrorExample, | ||
"Expected to find CustomError example with code 'CUSTOM_ERROR' and message 'Custom error occurred' " + | ||
"but service-level ValidationException appears to be overriding it"); |
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.
Instead of doing all of this manual assertion, let's just have the expected generated openapi file as a test resource and assert that it matches. Check AwsRestJson1ProtocolTest
to see how that's done.
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.
Done, tried to do it in a more focused, concise way just for this use case
.../src/test/java/software/amazon/smithy/openapi/fromsmithy/ErrorDeconflictingExamplesTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/software/amazon/smithy/openapi/fromsmithy/ErrorDeconflictingExamplesTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/software/amazon/smithy/openapi/fromsmithy/ErrorDeconflictingExamplesTest.java
Outdated
Show resolved
Hide resolved
…omsmithy/ErrorDeconflictingExamplesTest.java Co-authored-by: Miles Ziemer <[email protected]>
We noticed that when a Smithy service defines both service-level errors and operation-specific errors that map to the same HTTP status code, the generated OpenAPI spec includes the correct schema structure but completely omits error response examples.
Expected:
ErrorExamples from operation @examples should appear in HTTP error response sections.
Actual: Error response examples are missing while request and success examples work correctly.
This affects API documentation quality for services using both service-level and operation-specific errors.
This PR has the test case and a potential fix. Thanks!