joule-labs / webln

Spec and client library for WebLN apps and providers
244 stars 28 forks source link

Spec Proposal: Switch to ErrorCodes instead of pre-defined error classes #41

Open grunklejp opened 2 years ago

grunklejp commented 2 years ago

In the docs, we're told to check for a provider error based on the Error constructor value like so:

  if (err.constructor === MissingProviderError) {
    message = "Check out https://lightningjoule.com to get a WebLN provider";
  }

I think if we used an int instance property errorCode instead it would de-couple the webln spec from this specific implementation. We could call the property something like webLnErrorCode and have the values be an int.

Benefits to Providers

Benefits to webln supporting Apps

Rough mapping

Code Current Error message Description
0 MissingProviderError Thrown when requestProvider doesn't find a provider. This is a good one to catch to direct users to install an extension or browser that supports WebLN, especially if you have a favorite.
1 ~RejectionError~ UserRejectionError Thrown by providers when a user indicates that they don't want to complete a request from the application.
2 ConnectionError Thrown by providers when the node the provider use could not be reached for connection reasons (Either the user's network is down, or the node's network is down.)
3 UnsupportedMethodError Providers that only partially implement the WebLN spec can throw this error for methods they don't support.
4 RoutingError Thrown by providers when a node couldn't be routed to. This is a good time to prompt users to add your node as a peer, or open a channel.
6 InvalidDataError Thrown by providers if some data passed by the application is incorrect, such as a malformed BOLT-11 payment request.
7...98 Reserved Reserved for future error codes
99 InternalError A catch-all for errors that happened in the provider that the app can't really do anything about.

The best for last

wbobeirne commented 2 years ago

Agreed, a property on the error would be a much better solution overall.

0 - MissingProviderError

We should stick with something more unix like and make error codes > 0, especially since 0 is falsy. Your example of if (e.webLnErrorCode) wouldn't catch this otherwise.

grunklejp commented 2 years ago

We should stick with something more unix like and make error codes > 0, especially since 0 is falsy.

Absolutely, silly oversight on my part. I'm also curious if we should pass the underlying rpc error code (if there was one) to the consumer as well, much like MetaMask. I imagine this would be very difficult across multiple implementations.

wbobeirne commented 2 years ago

Yeah, I think between multiple implementations + custodial wallets, WebLN should avoid anything overly specific to the underlying Lightning node.

grunklejp commented 2 years ago

A helper library to convert node-specific error messages to WebLN error messages would be sick.