Skip to content
This repository has been archived by the owner on May 8, 2023. It is now read-only.

catch async error when connecting to network #42

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mattcasey
Copy link

When the Lit Protocol is down, there's no way for the user to catch the exception resulting in an unhandledPromiseRejection. This was crashing our application. Not sure if we would want to add some logging, or maybe add a timeout if the client never returns.

@mattcasey
Copy link
Author

Hi, I saw the original code got updated, but it doesn't look like the correct fix. Catch/throw an error will still result in crashing the process. It also makes it a bit more brittle, since any node connection error will crash the whole thing, whereas before it would resolve so long as a minimum amount of nodes were up. This is a simplified version of the code:

async function connect() {
  new Promise((resolve, reject) => {
    reject('network error!');
   })
  .then(() => {}) // happy path
  .catch(e => {
    throw e
  })
}

async function initClient () {
  try {
    await connect()
  } catch(error) {
    console.log('caught the error: ', error)
  }
}

initClient();

In order for the error to not crash the process, you need to await on each new promise (so adding await in front of new Promise() in my example). One problem with returning the error is that you would have to wait until all promises have resolved in some way. Currently I think it resolves as soon as minNodeCount is satisfied. Some ideas:

  • just swallow the error silently since it's not supposed to ever really happen anyway,
  • swallow but log the error, maybe when a DEBUG flag is enabled
  • collect any errors from each node, and once you have tried them all unsuccessfuly, reject the promise that is created on line 1067

@harbu
Copy link

harbu commented Jan 18, 2023

I agree the problem persists. In case of an error, there is no way for the user to catch it because it is thrown asynchronously de-facto.

Perhaps something like below would make the error catchable:

 async connect() {
    // handshake with each node
    await Promise.all(this.config.bootstrapUrls.map((url) => {
      return this.handshakeWithSgx({ url }).then((resp) => {
        this.connectedNodes.add(url);
        this.serverKeys[url] = {
          serverPubKey: resp.serverPublicKey,
          subnetPubKey: resp.subnetPublicKey,
          networkPubKey: resp.networkPublicKey,
          networkPubKeySet: resp.networkPublicKeySet,
        };
    })

    return new Promise((resolve) => { 
       // ... and so forth
    })
}

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

Successfully merging this pull request may close these issues.

2 participants