Broker spec doesn't specify HTTP status codes from consumer

[grantr]

Describe the bug The Broker spec isn't clear about what HTTP status codes consumers should use.

Expected behavior It is clear to readers which status codes should be used to indicate success or failure.

[lberk]

@grantr would you be willing to elaborate a bit more on the problem and which status codes the user was specifically looking for specification on? Perhaps a link to the original request?

[grantr]

This issue was prompted by this Slack thread: https://knative.slack.com/archives/C9JP909F0/p1601055597044200

@csantanapr asks:

Where in the docs I can find how the reply from a function work when is fire with trigger, I heard you can reply with CloudEvent. Where is documented with more details, I think I saw an issue about returning 204 or 200, and the body containing text or not, so if not replying with a 200 with CloudEvent, can I reply with a 201 with body include id, or cloudevent representing a future result, 200, 204 no body. TLDR what is the spec/contract on that response

The spec says callable sinks must reply with 200 and non-callable sinks must reply with 202. So which response code should a consumer use if it doesn't want to include an event in its response? Is this a callable sink or a non-callable sink?

I think the spec was written with the assumption that only intermediaries with a decoupled forwarding path like Broker and Channel are non-callable, because the event is not technically delivered to the end consumer by response time, so no reply is possible yet. Every other sink is technically callable even if it chooses not to reply with an event. So I suspect the answer to Carlos' question is, use code 200.

In practice, many sinks would be considered by their authors to be non-callable because they never reply with an event. I guess we could resolve this issue by being more clear that EVERY consumer is actually a callable, but then I wonder what is the value of the callable concept in the spec?

Related, why do we need to define a different response code that applies only to brokers and channels? Why are those entities required to respond with 202 when their contracts clearly state that by responding with success, they are taking responsibility for delivering the event, eliminating the publisher's need to follow up on the status of the delivery? Given these semantics, a 200 response makes more sense to me than 202.

[vaikas]

I kind of think that 202 is wrong and very difficult for us to reason about: https://tools.ietf.org/html/rfc7231#section-6.3.3

The request might or might not eventually be acted upon, 

Seems like 200/204 are the most reasonable response codes to expect:

Aside from responses to CONNECT, a 200 response always has a payload,
   though an origin server MAY generate a payload body of zero length.
   If no payload is desired, an origin server ought to send 204 (No
   Content) instead.  

[antoineco]

What sounds the most natural to me, and seems aligned with Ville's comment above:

• 204: ACK without event in the response.
• 200: ACK with an event / a batch in the response.

Alternative ACKs which are valid but shouldn't be treated differently by the Broker:

• 201: Same meaning as 200 but can be used by the consumer to indicate that it created a resource (example: a GitHub issue, a Zendesk ticket, ...), in which case I expect a description of that resource to be in the response as an event of the corresponding type (com.github.issue, com.zendesk.ticket, ...)
• 202: I believe this can be an acceptable ACK in a special case: when the destination simply forwarded/stored the message without actually processing it. The message was effectively consumed but we don't know if or when it will be acted upon.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[slinkydeveloper]

It seems to me this is an easy fix for whoever is interested, you can try to open a pr modifying the spec as @antoineco proposed and then we can reiterate from there

/kind good-first-issue

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.