Payload, Responder, and HTTP

[shadowhand]

I think the discussion in #13 and our own struggles with payload in sparkphp/spark#26 point to a disconnect between generating payloads and responses that has not been adequately satisfied.

I propose that there are only 5 possible "classes" of status:

• informational (HTTP 1xx)
• success (HTTP 2xx)
• redirect (HTTP 3xx)
• client/input error (HTTP 4xx)
• server/output error (HTTP 5xx)

I'm not quite sure what the implications of this are for Aura.Payload or Radar.Responder, but the main issue we (Spark) have with the current implementation is there are too many default Payload states to make it reasonable to add more by extending Payload, but too few to adequately express the full range of HTTP states.

As a starting point for resolving this, I would suggest some of the following options might be considered:

• removing all the default status types from Payload
• switch to integer types for the default statuses (this is what we did in Spark and it almost worked)
• address Payload status -> HTTP status in some other way (perhaps by requiring that all domains are packaged with a responder?)
• provide a full range of default statuses that can be directly mapped to HTTP status

[shadowhand]

I just had another thought while reading Avoid Hardcoding HTTP Status Codes:

Maybe what we really want is for Payload to mirror HTTP status text and use something like lukasoppermann/http-status to translate the status to a code in the responder?

For sake of completeness, this would look something like:

$payload->setStatus(Payload::OK);
$payload->setStatus(Payload::NO_CONTENT);
$payload->setStatus(Payload::SEE_OTHER);

While I don't particularly love the idea of coupling HTTP to the domain, this feels like the most practical and could easily be translated for command-line errors by providing a different translation table.

[pmjones]

A couple of notes:

1. The Payload statuses, such as they are, are intended primarily to indicate what happened in the Domain. They are supposed to be completely independent of HTTP status code. That set of provided Payload statuses looks very similar to HTTP statuses is coincidental; I attribute that to the wisdom of the HTTP spec writers being right about "the list of things that can happen in an interaction" way in advance.
2. I think the Payload statuses currently mirror the HTTP text strings, with some additions due to the fact that the Domain may be reporting on things that may not translate directly to a specific HTTP status (e.g., "Not Created").

Whether that's the correct approach or not may be debatable, but that does reflect the original intent. Let me know what you think of those two points, and I'll circle back around.

[pmjones]

As a side note, the Payload statuses being decoupled from HTTP statuses means they can be used in a CLI (and perhaps other) environments as well.

[elazar]

As a side note, the Payload statuses being decoupled from HTTP statuses means they can be used in a CLI (and perhaps other) environments as well.

Yes, please! :trophy:

[shadowhand]

They are supposed to be completely independent of HTTP status code

I understand that, but I am of the opinion that it causes more problems than it solves, hence this issue.

Payload statuses being decoupled from HTTP statuses means they can be used in a CLI

Nothing prevents the HTTP status text from being translated to a different set of codes for CLI usage.

Basically my point in all of this is that if the default Payload statuses happened to match up 100% with HTTP statuses text, it would be trivially easy to generate HTTP status codes from them and almost as easy to generate CLI status codes (by providing a separate translation table).

The current implementation is an awkward middle group in which neither situation is true and we end up with large amounts of code to handle a situation that could be almost entirely abstracted.

[pmjones]

The current implementation is an awkward middle group in which neither situation is true

A fair point. Since the current list of Payload statuses includes non-HTTP-ish statuses, do you favor discarding them? Or keeping them as ancillary/appendix/additional statuses?

[shadowhand]

I would prefer to either:

• sync Payload statuses to HTTP statuses (and remove all others)
• stop using Payload status and switch to an ancillary interface that provides the status

[pmjones]

So, for argument's sake, what do you think would be the right thing to when the Domain reports back that the operation failed because the requestor is not authenticated? (Which is different from being not authorized.) Since there is no HTTP status for "not authenticated" (among others), what should happen there?

[shadowhand]

@pmjones I think that is somewhat outside the scope of this discussion as it would require a customization of the Responder to correctly handle it. But more or less:

$payload->setStatus(Payload::FORBIDDEN);
$payload->setInput(/* some kind of identifier of the credentials? */);
$payload->setMessages(/* authentication error */);

[pmjones]

it would require a customization of the Responder to correctly handle it

(/me ponders)

And it seems to me that customizing the Responder for the kinds of Payloads it's going to handle is a reasonable thing. E.g., a BlogPostResponder might have a very different series of things to do than an AccountCreationResponder might.

[pmjones]

@shadowhand Further thoughts on this?

[shadowhand]

@pmjones I still think having the default Payload status match HTTP status text would be an improvement.

[elazar]

One of these might be useful?

shrikeh/teapot

lukasoppermann/http-status

[shadowhand]

Yes I mentioned the latter up above.

[elazar]

Ah, so you did. Oops. :flushed:

[elazar]

Related: https://philsturgeon.uk/http/2015/08/16/avoid-hardcoding-http-status-codes/

[shadowhand]

@elazar funny, I linked to exactly the same thing in the same comment.

[elazar]

@shadowhand D'oh! Apparently, I either need to learn to read or get a better memory.

[cxj]

I'm all in agreement with not hard-coding HTTP status codes, but I sure don't want my Domains coupled to, or even contaminated with HTTP. I'm a big CLI PHP builder, in addition to web apps. I want to re-use my code. Hence, I would object to "sync Payload statuses to HTTP statuses (and remove all others)" as a solution.

[pmjones]

After long consideration, I am declining the proposal to match Payload statuses to HTTP statuses. Having said that, recent changes in Payload and Payload_Interface may help to mitigate the stated problem. E.g., there is now a separate PayloadStatus class in Payload_Interface, so the statuses are no longer bundled into the Payload object itself.