Don't advance the consumer group when events can't be delivered.

[evankanderson]

Fixes reported issue in #eventing

We shouldn't advance a consumer group in Kafka until the downstream has accepted it, or we may drop events.

NOTE: no tests seemed to cover this case. I'm happy to try adding a test in a bit, but Alejandro Saucedo in slack wanted to see the change.

Proposed Changes

• Handle() is supposed to return true only when the message has been successfully delivered, but the channel hard-wired it to always return true, even if the delivery returned an error (failure).

Release Note

- 🐛 🧽 Fixed a case where the Kafka channel could drop messages if the recipient was unavailable.

Docs

No documentation changes needed, as the intended behavior of the Kafka channel was to not drop messages.

[evankanderson]

Note that the source doesn't have the same problem, because it does substantial error-checking:

https://github.com/knative/eventing-contrib/blob/master/kafka/source/pkg/adapter/adapter.go#L168

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-contrib-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
kafka/channel/pkg/dispatcher/dispatcher.go	66.7%	66.2%	-0.4

[n3wscott]

/lgtm
/approve

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, n3wscott

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• kafka/OWNERS [evankanderson,n3wscott]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment