Skip to content

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

Merged
n3wscott merged 3 commits intocloudevents:mainfrom
danielgrist:protocolresult
May 24, 2021
Merged

Adding error to protocol result if not 200 status code#686
n3wscott merged 3 commits intocloudevents:mainfrom
danielgrist:protocolresult

Conversation

@danielgrist
Copy link
Copy Markdown
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
Copy Markdown
Contributor

duglin commented May 17, 2021

@n3wscott could you take a look at this one?

result = protocol.ResultACK
} else {
result = protocol.ResultNACK
respbody, err := ioutil.ReadAll(resp.Body)
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@danielgrist danielgrist deleted the protocolresult branch May 29, 2025 16:34
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