Reconsider Piaf.Response error wrapping

[Kingdutch]

Problem

Currently Morph.Response.t is a result(Piaf.Response.t, failure). Failure is defined as

type failure = [
  Piaf.Error.t
  | `User(string)
  | `Server(string)
  | `Auth(string)
];

It's not entirely clear to me what the added value is of this extra result wrapping. Mostly because I'm unsure when I would receive a failure from Morph vs a response (a search showed my it's only really in the MSession middleware). As a simple test an incorrect query to a GraphQL server seems to return an internal server error (but not a Morph failure).

To illustrate the difficulty caused by the wrapping. If I want to output the status code of a response I need the following snippet.

let response_to_status : Morph.Response.t => int = (morph_res) => {
  (switch(morph_res) {
    | Ok(res) => res
    | Error(f) => f |> Morph.Response.response_of_failure
  }).status |> Piaf.Status.to_code
};

Suggestion

Remove the indirection around the response and make it a standard to use error responses for unrecoverable errors.

It's probably better to write tooling to help determine if a response is successful or not (if that's needed). Additionally, by always dealing in Piaf responses (or a thin, non-result wrapper with additions around that) we open up the possibility for Middleware to "save" a request using error correction.