fix: missing error message when return HTTPResult #785
Conversation
v2/protocol/http/protocol.go
Outdated
| @@ -390,6 +391,11 @@ func (p *Protocol) ServeHTTP(rw http.ResponseWriter, req *http.Request) { | |||
| } | |||
|
|
|||
| rw.WriteHeader(status) | |||
| if errMsg != "" { | |||
There was a problem hiding this comment.
I believe go does not like to write any body after writing the status header. Can you flip these lines?
There was a problem hiding this comment.
Thanks for your advice @n3wscott . It seems that we have to write body after writing the header. In Go source code https://github.com/golang/go/blob/dev.boringcrypto.go1.18/src/net/http/server.go#L1600, which shows that it will automatically set header when calls Write, and in https://github.com/golang/go/blob/dev.boringcrypto.go1.18/src/net/http/server.go#L1146, which shows that after any WriteHeader called, the flag will be set to true, and in https://github.com/golang/go/blob/dev.boringcrypto.go1.18/src/net/http/server.go#L1140, which checks the flag and if it was the true, the new StatusCode will be ignored. I was wondering if my understanding was right, hope your feedback. : )
Signed-off-by: wenfeng <[email protected]>
Signed-off-by: wenfeng <[email protected]>
Signed-off-by: wenfeng <[email protected]>
|
@n3wscott Hi, please have a look on the PR : ) |
| if res != nil { | ||
| var result *Result | ||
| switch { | ||
| case protocol.ResultAs(res, &result): | ||
| if result.StatusCode > 100 && result.StatusCode < 600 { | ||
| status = result.StatusCode | ||
| } | ||
|
|
||
| errMsg = fmt.Errorf(result.Format, result.Args...).Error() |
There was a problem hiding this comment.
I remember why we did not allow this, we need a way to let the integrator control if the error body is sent or not, some senders expect a cloudevent in response and this will not parse as a cloudevent as a result.
I am in favor of landing this as is but waiting for a release to add an option to control the output format here to let the integrator choose how this error is formatted on return.
fix issue #783