interledger / rfcs

Specifications for Interledger and related protocols
https://interledger.org
Other
430 stars 106 forks source link

Proposal: Define ILP Errors #150

Closed emschwartz closed 7 years ago

emschwartz commented 7 years ago

Right now we have the concept of a "rejection reason" when transfers are rejected, but those messages are neither well-defined nor relayed backwards by connectors.

We should define:

Here's a first stab at some error cases:

Connector rejected payment

Receiver rejected payment

These cannot necessarily be "trusted", because a connector relaying the message back could modify it, but it still seems helpful for development purposes at least.

adrianhopebailie commented 7 years ago

Do we want to include regulatory stuff? Maybe just a generic transfer not permitted or similar?

emschwartz commented 7 years ago

ILP 451

sharafian commented 7 years ago

I think ILP error messages should also have some indication of what component is rejecting the transfer. A rejecting coming from the ILP Kit API is different from a rejection coming from the ILP Connector, and in the case where both are listening on an account it can help diagnose problems.

adrianhopebailie commented 7 years ago

I think ILP error messages should also have some indication of what component is rejecting the transfer.

Where possible this would be great but I would be wary of making it a hard requirement until we are certain it can be done (i.e. the information is (and should be, privacy etc considered) available at the end-point where the error is handled)

emschwartz commented 7 years ago

From @justmoon:

Let's make sure all ILP rejections always include the ILP address and component type (connector, ledger, plugin, receiver, application-layer, app) of whatever component rejected it.

justmoon commented 7 years ago

@sentientwaffle wrote up a proposal: https://gist.github.com/sentientwaffle/ce5af28f16c26f35c453a7b7078e4edf

justmoon commented 7 years ago

I like in HTTP that we can look at the first digit of the error code to figure out what to do:

This is an error categorization that is actually useful!

I wonder if it should really be just a rejection mechanism or just a general status mechanism... maybe it'd be nice if I got a reply from the connector when they forwarded the payment. Might be too noisy, but could help debugging.

In our case, I can see the following categories (using @emschwartz' list in the original issue as a starting point):

This is not meant to final obviously. Let us know what you would add/remove/change!

earizon commented 7 years ago

I just updated a old (java) mini-library I've been using for years (and that has always worked really well for me) that can inspire another point of view about error control. It put the emphasis on the external entity that will be in charge of fixing the errors, instead of the source of the error. Once the code has enought information to know who must take charge it forgets (since actually the code can't make anything to fix the error). https://github.com/earizon/java_recipient_exceptions/blob/master/README.md I think adding an minimal codification to classify error codes (4xx, 5xx,....) is something useful, but the extra xx is usually not since such extra codes must in the end be interpreted by humans. A single english plain text (probably wrapped in a JSON object,....) is ussually more descriptive. Since errors are supposed to be the exception, not the norm, the extra bytes makes not much difference in terms of performance and give freedom to developers to trigger as much different errors as desired, with different degree of details, without being constrained to a previous API. (error codes can still be useful, but not mandatory). Also system administrators will apreciate not having to search in tons of manuals the meaning of an error code while the system is down and the storm approach. For GUI designers it will also be easier to just display on the screen the human text received, with no extra efforts to interpret the meaning of the error.

sappenin commented 7 years ago

Not sure if it's exactly something we want to duplicate, but there's some prior art (or at least, an attempt at prior art) when it comes to defining errors for HTTP Apis: https://tools.ietf.org/html/rfc7807

I'm not sure how popular it is, but at a prior company I worked at, we were exploring a move to this standard for our API error messages. There's a java implementation here that works quite well: https://github.com/zalando/problem

I understand the desire to de-couple ILP error codes from HTTP, so maybe a mapping from standard ILP error codes to HTTP error codes might be appropriate (or maybe a way to wrap ILP error codes in an HTTP/JSON format?)

emschwartz commented 7 years ago

I think we definitely want to decouple ILP errors from HTTP because much of the time they won't actually be sent over HTTP anyway.

We need some format for errors, like we needed some format for the ILP packet. However, the requirements for errors are a bit different than for the packet:

The options I see are:

For the HTTP-formatted headers + data, the idea is not to use HTTP error codes or to make it dependent on HTTP, but just to use the data format in the same way we're using it for PSK. Something like:

ILP/1.0 S00: Bad Request
Triggered-By: example.ledger.bob
Forwarded-By: example.connector.connie,example.connector.chloe
Date: 2017-03-23T00:00:00Z
Content-Type: application/json

{
  "id": "SomeApplicationError",
  "message": "Oops..."
}

Side note: I wonder if these errors should be thought of as "ILP errors", or being on the transport layer, or on an orthogonal plane (kind of like quoting a routing).