sdk-go icon indicating copy to clipboard operation
sdk-go copied to clipboard

HTTP Send example fails with SEGV on connection refused

Open lance opened this issue 3 years ago • 2 comments

When running the program in samples/http/sender, if the sender gets connection refused from the sink, a SIGSEGV occurs.

❯ ./sender
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x7d61fb]

goroutine 1 [running]:
main.main()
	/home/lanceball/src/github.com/cloudevents/sdk-go/samples/http/sender/main.go:48 +0x56b

The example code uses res.IsUndelivered() to check if the event has been delivered. Comments for IsUndelivered() say

// IsUndelivered true means the target result is not an ACK/NACK, but some other
// error unrelated to delivery not from the intended recipient. Likely target
// is an error that represents some part of the protocol is misconfigured or
// the event that was attempting to be sent was invalid.

It seems, client.Send() should be returning a plain error when getting connection refused, which is neither ACK nor NACK, but is not.

lance avatar Jun 16 '21 15:06 lance

Maybe @n3wscott can help you with this one?

slinkydeveloper avatar Jun 18 '21 06:06 slinkydeveloper

I noticed this too - the connection refused is wrapped in aReceipt here: https://github.com/cloudevents/sdk-go/blob/9f80fd3a004fcabea03987db1fdfcb324deef14e/v2/protocol/http/protocol_retry.go#L38

IsUndelivered checks if it's an ACK or NACK via errors.Is which checks the ACK boolean: https://github.com/cloudevents/sdk-go/blob/9f80fd3a004fcabea03987db1fdfcb324deef14e/v2/protocol/result.go#L103

Obviously bool can only be true or false, so any Receipt would always be detected as an ACK or NACK by IsUndelivered.

The doc for Receipt even says it's intended for ACK or NACK: https://github.com/cloudevents/sdk-go/blob/9f80fd3a004fcabea03987db1fdfcb324deef14e/v2/protocol/result.go#L78-L79

Therefore I believe the error shouldn't be a Receipt (Receipt should only be used if we get an actual HTTP status code back). If it's changed to any other instance of Result (which is just a type alias for error), then IsUndelivered would return true.

danieltmartin avatar Feb 24 '22 23:02 danieltmartin