stakwork / sphinx-relay

Node.js wrapper for communication between sphinx client and lightning node.
MIT License
247 stars 71 forks source link

Notify the app upon routing error #273

Open decentclock opened 2 years ago

decentclock commented 2 years ago

Currently, when sphinx relay gets back a routing error from its RPC interface to the node, the app does not get notified of this failure.

An example of the RPC error returned from the node:

Jan 17 15:20:15 raspberrypi sphinx-relay[4054]: [debug]: Client: Received message 23 from client
Jan 17 15:20:15 raspberrypi sphinx-relay[4054]: [lightning] [ 'keysendMessage' ]
Jan 17 15:20:15 raspberrypi sphinx-relay[4054]: [lightning] [ 'keysend' ]
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]: KEYSEND ERROR Error: 13 INTERNAL: RPC error response: RpcError { code: 210, message: "Ran out of routes to try after 1 attempt: see `paystatus`", data: None }
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:     at Object.exports.createStatusError (/home/pi/sphinx-relay/node_modules/grpc/src/common.js:91:15)
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:     at Object.onReceiveStatus (/home/pi/sphinx-relay/node_modules/grpc/src/client_interceptors.js:1209:28)
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:     at InterceptingListener._callNext (/home/pi/sphinx-relay/node_modules/grpc/src/client_interceptors.js:568:42)
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:     at InterceptingListener.onReceiveStatus (/home/pi/sphinx-relay/node_modules/grpc/src/client_interceptors.js:618:8)
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:     at callback (/home/pi/sphinx-relay/node_modules/grpc/src/client_interceptors.js:847:24) {
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:   code: 13,
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:   metadata: Metadata { _internal_repr: { date: [Array] }, flags: 0 },
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]:   details: 'RPC error response: RpcError { code: 210, message: "Ran out of routes to try after 1 attempt: see `paystatus`", data: None }'
Jan 17 15:20:17 raspberrypi sphinx-relay[4054]: }
Jan 17 15:20:18 raspberrypi sphinx-relay[4054]: [lightning] [ 'listChannels' ]
Jan 17 15:20:18 raspberrypi sphinx-relay[4054]: [lightning] [ 'listChannels' ]
Jan 17 15:20:18 raspberrypi sphinx-relay[4054]: [lightning] [ 'pendingChannels' ] Jan 17 15:20:48 raspberrypi sphinx-relay[4054]: [lightning] [ 'listChannels' ]

I had a chat with @tomastiminskas, and he believes there are two options:

  1. Return an error message with the response to the API call that triggers the routing in the first place.
  2. Add a socket message from relay to the app that is sent upon routing failures such as the one above.

Let me know if you have any questions / need any clarifications.

tomastiminskas commented 2 years ago

The socket message from relay to app when that happens would be a first simple solution, but that would work just when the app is active and the socket is connected.

Ideally, as a long term solution, relay should have more descriptive error messages on all endpoints or defined error codes, and app should just show that error coming from relay to the user on a toast.

kevkevinpal commented 2 years ago
1. Return an error message with the response to the API call that triggers the routing in the first place.

2. Add a socket message from relay to the app that is sent upon routing failures such as the one above.

Should we maybe get a socket message out first and then also have there be an error response as well in the future?

I can start looking into how to get a socket message for this setup

kevkevinpal commented 2 years ago

seems like this is where that error is being thrown

https://github.com/stakwork/sphinx-relay/blob/master/src/network/send.ts#L205

kevkevinpal commented 2 years ago

@tomastiminskas how are we handling 400 errors right now? do we tost the error field if success is false?

Evanfeenstra commented 2 years ago

seems like this is where that error is being thrown

https://github.com/stakwork/sphinx-relay/blob/master/src/network/send.ts#L205

ya theres a "failure" callback so the higher level function can pass back the error https://github.com/stakwork/sphinx-relay/blob/master/src/network/send.ts#L211

So for example for sending a simple text message, we need to add a "success" and "failure" functions here https://github.com/stakwork/sphinx-relay/blob/master/src/controllers/messages.ts#L363. The complicated part is that in tribes, "sendMessage" can actually send a bunch of messages, that can both be over MQTT and over Lightning

tomastiminskas commented 2 years ago

@kevkevinpal sorry I missed this. So the app just shows a yellow/red bolt in dashboard if any request returns a code other than 200. And at the moment the endpoint returns failure we are on most of the cases showing a generic error alert like:

"action failed, please try again later"

If relay starts returning descriptive error message on each endpoint the app can start showing the error coming from relay to the user. Then giving support to users when something is failing will be much easier since we will know the cause of the problem.

In addition, If socket message is implemented I can easily show a toast or alert with the message coming from relay through socket. Each socket message has a type (we can create a new type error and then inside the response send a message that will be directly shown to the user.

kevkevinpal commented 2 years ago

So for anyone taking up this issue a good example to look at is to learn how we use websockets is for when relay receives a message

Here the correct function is being called when we get the message https://github.com/stakwork/sphinx-relay/blob/master/src/controllers/index.ts#L178

The socket io message being sent in that above function https://github.com/stakwork/sphinx-relay/blob/master/src/controllers/messages.ts#L462=

and we'd want to do something similar for errors right @tomastiminskas maybe a

socket.sendJson(
    {
      type: 'error',
      response: jsonUtils.errorToJson(...),
    },
    tenant
  )  
Evanfeenstra commented 1 year ago

I think we can return the error directly as the result of the endpoint that sent the message

Evanfeenstra commented 1 year ago

In fact the errorMessage should be stored directly on the Message table itself, so you can see the errors after the fact as well