Some exceptions thrown by web3-eth-account cannot be caught

[kai-alpha]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

Accounts.signTransaction() can generate a stray Promise under JS job queue, and there's no way for library users to catch any exceptions the stray promise throws, which can kill the entire Node.js process. A library shouldn't allow this to happen.

Expected Behavior

try { ... await account.signTransaction(tx) } catch (e) { ... } should be able to catch any exceptions thrown (conceptually) within signTransaction() method

Steps to Reproduce

Set tx.maxPriorityFeePerGas but not maxFeePerGas and send it to BSC (or any EVM-compatible blockchains without EIP-1559) to hit this throw at
https://github.com/ChainSafe/web3.js/blob/2b7c50a4cd27e5b21def779dd757cdacd2afd31a/packages/web3-eth-accounts/src/index.js#L433

Yes I know that doesn't make sense at all, but that's another story. A bug in our quick & dirty sandbox script led me to the issue.

Web3.js Version

1.7.0

Environment

No response

Anything Else?

_handleTxPricing() runs something like

return new Promise((resolve, reject) => {
  try {
    Promise.all([two async REST calls]).then(
      ...use the return value of the two REST calls...;
      resolve(suggested values for maxPriorityFeePerGas and maxFeePerGas))
  } catch (e) { reject(e) }
}

to asynchronously return the suggested values with resolve(), but I think the problem is that any exceptions thrown within the inner Promise.all() (and its then() chain) aren't propagated to reject() from the outer Promise.

Here is what I believe is a minimal repro: TS playground

JS isn't among my core expertise, I can't use JS Promise without async/await so this is all I can share.

Ping @spacesailor24 according to Git history

[nazarhussain]

Thanks @kai-alpha for reporting the issue. We will look into to investigate further.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[kai-alpha]

wut, this is a real bug

[kristjanpeterson1]

Are there any updates on this? Seems like a very serious issue..

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[kai-alpha]

Hmm nobody is fixing this bug? Last attempt to get attention (I'll let the bot close it next time)

[andreistefanwork]

Hi @kai-alpha! I was able to reproduce this issue as well. The problem seems to be indeed inside the inner Promise.all([promise1, promise2]) and its then() chain.

If an error occurs in either promise1, promise2 or inside then(...), it's not propagated to the outer Promise

This can be fixed using a catch(...) on the inner Promise.all()

I created the following PR, in which I also included a test to cover this change.

Can you double check everything looks alright?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

[TangMonk]

Still problem?

[jdevcs]

@TangMonk @kai-alpha are you still able to reproduce ?

[cocacola-lover]

It still happens on 4.6.0

(async function test() {
  try {
    await web3.eth.sendSignedTransaction("")
  } catch (e) {
    console.log(e)
  }
})()

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.