Finish() no longer used binding-message in Send()

[xinydev]

Finish() no longer used binding-message in Send(). and I found that some trace mechanisms must rely on Finish() to complete.

Signed-off-by: XinYang xinydev@gmail.com

[slinkydeveloper]

I think this PR is wrong, finish is invoked by the cloudevents.Client. If you're using the protocol manually, you should invoke finish by yourself

[xinydev]

I think this PR is wrong, finish is invoked by the cloudevents.Client. If you're using the protocol manually, you should invoke finish by yourself

yes, when the user invokes Request() function in cloudevents.Client, the Finish() is invoked by cloudevents.Client.

but when the user invokes Send() function in cloudevents.Client, the cloudevents.Client can not get the original binding.Message, the binding.Message only exists in protocol's Send() function.

[slinkydeveloper]

but when the user invokes Send() function in cloudevents.Client, the cloudevents.Client can not get the original binding.Message, the binding.Message only exists in protocol's Send() function.

Maybe that's what needs to be fixed? I mean, on the cloudevents.Client side?

[xinydev]

but when the user invokes Send() function in cloudevents.Client, the cloudevents.Client can not get the original binding.Message, the binding.Message only exists in protocol's Send() function.

Maybe that's what needs to be fixed? I mean, on the cloudevents.Client side?

We can indeed fix this on the cloudevents.Client side, by changing this to invoke protocol's Request() function .

But the problem still exists, users who use the protocol manually, they invoke the protocol's Send() function, and then, they can not invoke the Finish() because the Send() function does not return the Message

The msg in L143 no longer accessible outside the protocol's send function, and we need to change the protocol's interface if we want to finish this msg out of this function.

sdk-go/v2/protocol/http/protocol.go

Lines 136 to 156 in 43b8eca

	func (p *Protocol) Send(ctx context.Context, m binding.Message, transformers ...binding.Transformer) error {
	if ctx == nil {
	return fmt.Errorf("nil Context")
	} else if m == nil {
	return fmt.Errorf("nil Message")
	}
	msg, err := p.Request(ctx, m, transformers...)
	if err != nil && !protocol.IsACK(err) {
	var res *Result
	if protocol.ResultAs(err, &res) {
	if message, ok := msg.(*Message); ok {
	buf := new(bytes.Buffer)
	buf.ReadFrom(message.BodyReader)
	errorStr := buf.String()
	err = NewResult(res.StatusCode, "%s", errorStr)
	}
	}
	}
	return err
	}

[xinydev]

@slinkydeveloper Would you please take another look?
The Message I finished is the response of the HTTP request, not the body of the request. I think the user who uses Send() function does not care about this response Message(if they care response, they should call Request() instead), they only care about the request's success or failure.

sdk-go/v2/protocol/http/protocol.go

Lines 143 to 156 in 43b8eca

	msg, err := p.Request(ctx, m, transformers...)
	if err != nil && !protocol.IsACK(err) {
	var res *Result
	if protocol.ResultAs(err, &res) {
	if message, ok := msg.(*Message); ok {
	buf := new(bytes.Buffer)
	buf.ReadFrom(message.BodyReader)
	errorStr := buf.String()
	err = NewResult(res.StatusCode, "%s", errorStr)
	}
	}
	}
	return err
	}

[slinkydeveloper]

@n3wscott any ideas about this one?

[xinydev]

@n3wscott ping

[dan-j]

Looking at a few of the other protocols, they all set up a deferred call to Finish(), but the http protocol doesn't. Maybe this PR is correct?

[dan-j]

Ah, ignore my last comment, I misread the PR and just realised you explained you'd finished the response, not the original message (which is already finished)

I can't really comment on this either, I've never used the http protocol before so I don't know if this is going to cause any issues with other integrations.