Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
grunklejp opened this issue Jan 29, 2022 · 4 comments

Comments

@grunklejp
Copy link

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

  • Can more easily write & throw their own error messages
  • It's much more explicit & 'spec-like' so providers will feel more compelled to meet this requirement (looking at you Alby... 😠 )

Benefits to webln supporting Apps

  • ❗️Apps can now more effortlessly know if an error was thrown by the provider
catch(e){
  if (e.webLnErrorCode) {
    // we know that its a webln related  error
  }
}
// Instead of checking against every single webln error class
  • No longer have to import a bunch of Error classes to know if the provider threw an error

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

  • We can do this spec upgrade in a non-breaking way.
@wbobeirne
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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

@grunklejp
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants