Skip to content
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

Adding error to protocol result if not 200 status code #686

Merged
merged 3 commits into from
May 24, 2021

Conversation

danielgrist
Copy link
Contributor

Adding the error message to the protocol result so it can be retrieved when there are non 200 status codes.

Signed-off-by: Daniel Grist dgrist@ibm.com

@duglin
Copy link
Contributor

duglin commented May 17, 2021

@n3wscott could you take a look at this one?

@@ -42,7 +43,12 @@ func (p *Protocol) doOnce(req *http.Request) (binding.Message, protocol.Result)
if resp.StatusCode/100 == 2 {
result = protocol.ResultACK
} else {
result = protocol.ResultNACK
respbody, err := ioutil.ReadAll(resp.Body)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the body here will cause line 54 to error I think, because the body will have already been read.

The body of the response is already returned within the message, this change should not be required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was made because the message is dropped later on here: https://github.com/cloudevents/sdk-go/blob/main/v2/protocol/http/protocol.go#L142

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we read and wrap the returned message from Request in Send and return it if err is not nil on L142? So fix the problem one bump up, rather than in protocol_retry. Seems like protocol_retry is doing the right thing, we just need to not drop the body if there is an error in Send.

Signed-off-by: Daniel Grist <dgrist@ibm.com>
Signed-off-by: Daniel Grist <dgrist@ibm.com>
Signed-off-by: Daniel Grist <dgrist@ibm.com>
@n3wscott n3wscott merged commit 43b8eca into cloudevents:main May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants