Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Frontend error handling #388

Closed ferostabio closed 1 year ago

ferostabio commented 1 year ago

We want to get rid of Snackbar and replace with blocknative's notify library, now part of `@web3-onboard/core", which we already use. The main feature of this library is that it handles transaction notifications out of the box, removing the burden of having to do so manually.

Documentation is here.

transactionHandler: transaction => {
  console.log({ transaction })
  if (transaction.eventCode === 'txPool') {
    return {
      type: 'success',
      message: 'Your transaction from #1 DApp is in the mempool',
    }
  }
}

Started implementing here.

ferostabio commented 1 year ago

The best way to use the package is through @web3-onboard/react's useNotifications hook as described here.

Now, have a peek at the documentation, there are two things that worry me.

This hook allows the dev to access all notifications if enabled, send custom notifications and update notify <enable/disable & update transactionHandler function> note requires an API key be added to the initialization, enabled by default if API key exists For full Notification documentation please see Notify section within the @web3-onboard/core docs

and

Notify provides by default transaction notifications for all connected wallets on the current blockchain. When switching chains the previous chain listeners remain active for 60 seconds to allow capture and report of an remaining transactions that may be in flight. By default transaction notifications are captured if a DAppID is provided in the Onboard config along with the Account Center being enabled. An object that defines whether transaction notifications will display (defaults to true if an API key is provided). This object contains an enabled flag prop and an optional transactionHandler which is a callback that can disable or allow customizations of notifications. Currently notifications are positioned in the same location as the account center (either below, if the Account Center is positioned along the top, or above if positioned on the bottom of the view). The transactionHandler can react off any property of the Ethereum TransactionData returned to the callback from the event (see console.log in example init). In turn, it can return a Custom Notification object to define the verbiage, styling, or add functionality.

This means, first, that we can only use it to track the current chain. For most of our operations, this is trouble; we would have to rely on our history.store and check if the chain is the current one and, if not, show custom notifications. And if I understand correctly, it will happen a lot of times.

The second issue is that we require an API key. I don't think we should continue implementation until figuring this out.

ferostabio commented 1 year ago

Since notify library works for only one chain, it would still force us to handle things manually on our side for cross-chain operations.

We have then decided to improve our error handling without using they're built-in tx tracking feature.

@NikolaiYurchenko this is the new expected implementation. You can continue from the branch since I already removed snackbar (the store was pretty bad), there's a NotificationSomething component and there's an example of how to show custom notifications.

ferostabio commented 1 year ago

@NikolaiYurchenko we have some other tasks I already assigned to you, so let me continue with this one in the meantime.

ferostabio commented 1 year ago

@brozorec I've been looking at @web3-onboard/react/notify; if you don't use their out-of-the-box tx handling, it's pretty bad. Limited, no customization, it forces you to update some stuff even without using it... no update when it's dismissed unless you click on it, it's a bit poor. I didn't like Snackbar either. What if we use something like react-toastify?

ferostabio commented 1 year ago

Closing since we're not going to use notify after all and it's then a duplicate of https://github.com/Fujicracy/fuji-v2/issues/381.