elixir-omg icon indicating copy to clipboard operation
elixir-omg copied to clipboard

Refine & document Watcher's nested error responses

Open pnowosie opened this issue 4 years ago • 2 comments

Work on client libs reveal problems which is related to not handling messages field of the client_error message.

:exclamation: Please mind that client can be generated out from swagger spec - swagger should reflect correct error response (right now we only present HTTP 200 (success) & 500 (server_error). This not cover the client errors HTTP 200 (success: false) responses

Also (btw) check the status of #739

Slack discussion bellow:


image

Papa Paweł 1 hour ago

@jarindrian the unauthorized_spend I'm pretty sure messages is an array. Is :point_up: working?

jarindrian the unauthorized_spend 1 hour ago

will check :ok_hand::skin-tone-6:

piotr 1 hour ago

OK, thanks, I'll stop pestering you about this now, Paweł :slightly_smiling_face:. Or maybe not... I mean, maybe we should consider putting the more relevant part of the error (e.g. unauthorized_spend) into the more visible field :thinking_face:. It seems like 2 clients have missed the info sitting in messages. (edited)

piotr 44 minutes ago

or maybe at least provide a hint "look in the messages field for details", in the description field. Dunno, @Pong I'll defer to you to decide, whether this can be improved on elixir-omg's end somehow or not.

Papa Paweł 44 minutes ago

@jarindrian the unauthorized_spend messages is anything that thrown by the failing part so in this case it prblby isn't array :facepalm2:

Papa Paweł 42 minutes ago

@piotr I think this approach is correct - but we could improve and highlight the messages existence. it's not so popular than errors from the "first hand"

Pong 2 minutes ago

So I can speak on plasma-cli, since its golang and strict typing, I tell it to parse into struct as is shown in the error message on swagger for some reason, this is missing all the valuable message and only show `client_error

Pong < 1 minute ago

A more useful message in general is :heart: . I'd be supportive for this to provide more useful hint to client on what went wrong

pnowosie avatar Nov 07 '19 12:11 pnowosie

I'm not sure whether messages is an appropriate name though. :thinking: Watcher just passes anything it gets from ch-ch with it's :client_error message

This gets handled in the fallback ctr

In this particular case messages field isn't a list :confused:

pnowosie avatar Nov 07 '19 13:11 pnowosie

Also, as we followed Http-Rpc - these client errors are returned with HTTP 200 status code. Wouldn't be it a problem to show 2 messages in swagger (1 succesful + 1 client error) for the same status code? (CC @mederic-p )

pnowosie avatar Nov 07 '19 13:11 pnowosie