Add X-Amzn-Query-Error for error responses in JSON protocol#1191
Add X-Amzn-Query-Error for error responses in JSON protocol#1191
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds the X-Amzn-Query-Error header to error responses when using the JSON protocol (AWS JSON Protocol 1.0) in ElasticMQ's SQS REST API implementation. This header provides the error type to clients, which aligns with AWS SQS behavior for JSON protocol error responses.
Changes:
- Added custom header implementation
X-Amzn-Query-Errorfor Pekko HTTP - Modified error handling to include the header for JSON protocol error responses
- XML protocol (AWSQueryProtocol) error responses remain unchanged
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case _ => | ||
| respondWithHeader(`X-Amzn-Query-Error`(e.errorType)) { | ||
| complete(e.httpStatusCode, ErrorResponse(e.errorType, e.message)) | ||
| } |
There was a problem hiding this comment.
The new X-Amzn-Query-Error header added for JSON protocol error responses is not covered by any tests. Consider adding test coverage to verify that:
- The header is present in JSON protocol error responses
- The header contains the correct error type value
- The header is NOT present in XML protocol (AWSQueryProtocol) error responses
Example test location: rest/rest-sqs/src/test/scala/org/elasticmq/rest/sqs/directives/UnmatchedActionRoutesTest.scala already tests JSON protocol errors at line 66-69, and could be extended to check for the presence of this header.
| case _ => | ||
| respondWithHeader(`X-Amzn-Query-Error`(e.errorType)) { | ||
| complete(e.httpStatusCode, ErrorResponse(e.errorType, e.message)) | ||
| } |
There was a problem hiding this comment.
The pattern matching uses a catch-all case _ which will apply the X-Amzn-Query-Error header to any non-AWSQueryProtocol case. While this currently only matches AWSJsonProtocol1.0, consider being explicit about which protocols should receive this header to prevent unintended behavior if additional protocols are added in the future.
Alternative approach: Use an explicit match case for AWSProtocol.AWSJsonProtocol1.0 instead of the catch-all pattern.
| case _ => | |
| respondWithHeader(`X-Amzn-Query-Error`(e.errorType)) { | |
| complete(e.httpStatusCode, ErrorResponse(e.errorType, e.message)) | |
| } | |
| case AWSProtocol.`AWSJsonProtocol1.0` => | |
| respondWithHeader(`X-Amzn-Query-Error`(e.errorType)) { | |
| complete(e.httpStatusCode, ErrorResponse(e.errorType, e.message)) | |
| } | |
| case _ => | |
| complete(e.httpStatusCode, ErrorResponse(e.errorType, e.message)) |
closes #1188