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

Parsing CloudEvent from HTTP response for 4xx,5xx status codes. #558

Closed
matejvasek opened this issue Jul 27, 2020 · 8 comments · Fixed by #559
Closed

Parsing CloudEvent from HTTP response for 4xx,5xx status codes. #558

matejvasek opened this issue Jul 27, 2020 · 8 comments · Fixed by #559

Comments

@matejvasek
Copy link
Contributor

matejvasek commented Jul 27, 2020

Spec states Events can be also be transferred in HTTP responses and with all HTTP status codes that permit payload body transfers. However when I do request that results in 500:

receivedEvt, resp := cloudEventClient.Request(context.Background(), *evtToSend)

only error is set (correctly to "500:"), the receivedEvt is nil, despite the fact that response contained body with CloudEvent.

@matejvasek
Copy link
Contributor Author

matejvasek commented Jul 27, 2020

@n3wscott wdyt? Is it a bug?

@n3wscott
Copy link
Member

This should work if the headers are correct as well, can you make a little demo or paste the raw http body of the response?

@matejvasek
Copy link
Contributor Author

Response as seen in WireShark:

HTTP/1.1 500 Internal Server Error
Date: Mon, 27 Jul 2020 10:43:55 GMT
Content-Type: application/cloudevents+json; charset=utf-8
Server: Kestrel
Transfer-Encoding: chunked

{
  "specversion": "1.0",
  "type": "core.errorresponse",
  "source": "/of",
  "id": "de66d47f-6521-4b86-bc04-0b421e41367b",
  "time": "2020-07-27T10:43:55.9400808Z",
  "data": "{\"ErrorCode\":null,\"ErrorDetails\":\"Internal runtime failure. Please refer to runtime logs for details\"}",
  "datacontenttype": "application/json"
}

@matejvasek
Copy link
Contributor Author

@n3wscott I think that this may be deliberate:
somewhere in the code:

	var result protocol.Result
	if resp.StatusCode/100 == 2 {
		result = protocol.ResultACK
	} else {
		result = protocol.ResultNACK
	}

then later

	if !protocol.IsACK(err) {
		return nil, err
	}

but doesn't that go against spec Events can be also be transferred in HTTP responses and with all HTTP status codes that permit payload body transfers. ?

@n3wscott
Copy link
Member

in the case of the above event and the caller code of:

receivedEvt, resp := cloudEventClient.Request(context.Background(), *evtToSend)

I would expect resp to be a NACK, but also receivedEvt to be the full event in response. Let me poke around...

@n3wscott
Copy link
Member

YES, it is a simple fix.. 😅 one sec

@n3wscott
Copy link
Member

Thanks for the bug @matejvasek !! The simple fix is this: https://github.com/cloudevents/sdk-go/pull/559/files#diff-4fd5b8ce98982998f08e4be1a9c74c50R152

Give it a try locally and see if it solves your issue.

@matejvasek
Copy link
Contributor Author

Sorry for late reply. Thanks @n3wscott , it works!

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 a pull request may close this issue.

2 participants