-
Notifications
You must be signed in to change notification settings - Fork 39
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
Breaking change to websocket API: setup callback can report body of response #409
Conversation
…sting signature, making downstream fixup easier
Codecov ReportBase: 79.32% // Head: 79.31% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
==========================================
- Coverage 79.32% 79.31% -0.01%
==========================================
Files 27 27
Lines 11761 11759 -2
==========================================
- Hits 9329 9327 -2
Misses 2432 2432
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
int error_code; | ||
struct aws_websocket *websocket; | ||
const int *handshake_response_status; | ||
const struct aws_http_header *handshake_response_header_array; |
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.
Why not use const struct *aws_http_headers
? Do we really need to have a deep copy of the headers to an array here?
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.
I actually did that, and then reverted it.
There were some downstream uses where the array was being forwarded along, so they would need to deconstruct it back into a raw array to preserve their own APIs.
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.
and it's not a "deep copy" it's just one allocation long enough to hold an array.
it's still using the underlying strings stored in the aws_http_headers struct
Co-authored-by: Dengke Tang <dengket@amazon.com>
Adapt to changes from awslabs/aws-c-http#409
Adapt to changes from awslabs/aws-c-http#409
Issue:
If the server rejected a websocket connection, there was no way to see the body of the response. The response body often has useful information explaining why the connection was rejected.
Underlying cause:
The websocket code would close the connection immediately after receiving headers, because that was the simplest way to do things.
Description of changes:
Change the setup callback signature, so that the body can be reported.
The setup code keeps the connection open while receiving a rejection, making a copy of the body, and ultimately reporting it in the setup callback.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.