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

[ethers-v5] Add initial support for custom errors #682

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

fredlacs
Copy link

@fredlacs fredlacs commented May 1, 2022

This PR adds initial support for handling custom errors with Typechain.
Still WIP but opening a draft PR to be able to get input from others on the approach (currently heavily inspired on #457).

The idea here is to be able to handle errors in js/ts more cleanly when interacting with the chain.
Although the functionality isn't here yet, it would be cool to be able to use these to have cleaner try catches when doing interactions with ethersjs

The current setup returns "undefined" for the error objects but is inspired in this. A couple alternatives on how this could work (ie could rely on ethers and upstream something there or could just have a separate entrypoint). That said, I think only the typed error object exports themselves are already useful, could also have a utility function to get the signature / encode expected values

@changeset-bot
Copy link

changeset-bot bot commented May 1, 2022

⚠️ No Changeset found

Latest commit: c29ed32

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@fredlacs
Copy link
Author

fredlacs commented May 2, 2022

I think what we'd want here is to cast the ErrorFragment to a typed version of the error

@krzkaczor
Copy link
Member

Related: #667

Copy link
Member

@krzkaczor krzkaczor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable. I am personally not a huge fan of custom errors and furthermore, ethersjs support is kinda minimal but the PR looks good!

Can you try fixing CI?

@shashidhar771892
Copy link

CI test(14.x) the test was failed
Failing after 4m — test (14.x, ubuntu-latest

@frontier159
Copy link

This looks reasonable. I am personally not a huge fan of custom errors and furthermore, ethersjs support is kinda minimal but the PR looks good!

@krzkaczor out of interest - what's the reasoning for not being a fan of custom errors? It feels like they provide a better opportunity to handle errors in clients, and can also reduce bytecode in deployed contracts. But i'm interested in any downsides too before using in a project

@shashidhar771892
Copy link

Error: Process completed with exit code 1.
We can add the 56s in the CI / Test (14.x ubuntu-latest)

@wottpal
Copy link

wottpal commented Aug 22, 2022

This would be an awesome improvement for typechain and the web3 developer experience overall IMO. Really looking forward to see this one merged. Would also be interested in arguments against it, @krzkaczor.

@GeraldHost
Copy link

Would love to see this merged, will unblock an issue with hardhat not having access to the errors in the interface provided by typechain and therefore not being able to use assertion helpers such as revertedWithCustomError in the chain helpers plugin they provide.

@KholdStare
Copy link

@fredlacs @krzkaczor Is this pull request abandoned? Anything others can do to take it over the finish line?

@tfalencar
Copy link

tfalencar commented Nov 16, 2022

+1 for this feature. Can someone please re-trigger CI? The logs for last CI run are too old and disappeared.

@CJ42
Copy link

CJ42 commented Mar 31, 2023

Having support for custom error in Typechain would be indeed very useful.
Would be great if someone could re-trigger the CI :)

@KholdStare
Copy link

@fredlacs @krzkaczor Poking again, since it's been more than a year since this PR has been posted and issue #667 has been raised.

@emretepedev
Copy link

+1 for this feature.

@Alivers
Copy link

Alivers commented Jan 9, 2024

Any updates?

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

Successfully merging this pull request may close these issues.