fix: Handle timeout error in safe creation #1293
Conversation
Branch preview✅ Deploy successful! https://fix-safe-creation-error--webcore.review-web-core.5afe.dev |
src/hooks/useIsValidExecution.ts
Outdated
| @@ -11,7 +11,7 @@ import ContractErrorCodes from '@/services/contracts/ContractErrorCodes' | |||
| import { sameAddress } from '@/utils/addresses' | |||
|
|
|||
| const isContractError = <T extends EthersError>(error: T): error is T & { reason: keyof typeof ContractErrorCodes } => { | |||
| return Object.keys(ContractErrorCodes).includes(error.reason) | |||
| return Object.keys(ContractErrorCodes).includes(error.reason!) | |||
There was a problem hiding this comment.
I didn't bother with this as it will be removed with #1285 Actually not so we should handle it better.
There was a problem hiding this comment.
I would suggest something along the lines of:
const isEthersError = <T extends Error | EthersError>(error: T): error is EthersErrorThere was a problem hiding this comment.
Why does the function need to handle Error and EthersError if it is only used in one place? I think we should rework EthersError in general. Ethers v6 will add proper error types but for the time being I would suggest opting for a more simple solution e.g.
const isContractError = (error: EthersError) => {
if (!error.reason) return false
return Object.keys(ContractErrorCodes).includes(error.reason)
}
There was a problem hiding this comment.
The reasoning behind the union was because a standard error could be caught. Let's keep your solution and adjust the type as suggested in the future.
ESLint Summary View Full Report
Report generated by eslint-plus-action |
src/hooks/useIsValidExecution.ts
Outdated
| @@ -11,7 +11,7 @@ import ContractErrorCodes from '@/services/contracts/ContractErrorCodes' | |||
| import { sameAddress } from '@/utils/addresses' | |||
|
|
|||
| const isContractError = <T extends EthersError>(error: T): error is T & { reason: keyof typeof ContractErrorCodes } => { | |||
| return Object.keys(ContractErrorCodes).includes(error.reason) | |||
| return Object.keys(ContractErrorCodes).includes(error.reason!) | |||
There was a problem hiding this comment.
I would suggest something along the lines of:
const isEthersError = <T extends Error | EthersError>(error: T): error is EthersError
|
CLA Assistant Lite All Contributors have signed the CLA. |
|
The error is there. |
What it solves
Resolves #1291
How this PR fixes it
receiptso we should handle it as an optional typeOther changes
receiptfromTxEvent.PROCESSEDandTxEvent.REVERTEDas it was not usedHow to test it
Screenshots