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

fix: prevent silent failing during contract creation #59

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

peetzweg
Copy link
Collaborator

While using the hooks I experience serveral times that my contract instance from useContract or useRegisteredContract was plainly undefined.

The creation of the contract or fetching the deployment information just failed silently. This PR is meant to make it more transparent to the user if this happens.

Copy link

changeset-bot bot commented Jan 24, 2024

🦋 Changeset detected

Latest commit: 7865a62

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

@wottpal wottpal changed the base branch from main to develop January 25, 2024 01:41
@wottpal
Copy link
Member

wottpal commented Jan 25, 2024

Makes a lot of sense! Do you have an idea where these "failures" are coming from? It's still happening even though everything is defined correctly, api is initialized, etc.? Maybe something like await api.isReady should also be incorporated?

Also, is it happening in ink!athon for you (never really experienced it, but i most often have useEffects on whether the contract changes & check there is it's != undefined.

@peetzweg
Copy link
Collaborator Author

Ah forgot to mentioned how to reproduce a silent failure. It's just if you request a contractId which is not defined in the deployments. So only registered "ContractA" but requested later through useRegisteredContract("ContractB"). This will just return undefined without any error message. As undefined, undefined is passed to useContract which just does nothing if the address is undefined.

https://github.com/scio-labs/use-inkathon/pull/59/files#diff-6b56c5db493267a4ecbae1fbcd9fc1708bfe35539a290b0ad0145788ffd914fbL13-L14

Would have been nice to add some testcases for tese cases to show how they are handled. I started yesterday on creating a pr for basic vitest setup, but seems we need to adapt the Provider a bit to be able to pass along own wallets to pass along a mocked version for testing.

https://github.com/peetzweg/use-inkathon/blob/pz/vitest-setup/src/provider.test.tsx

@wottpal wottpal merged commit 4c339d8 into scio-labs:develop Feb 5, 2024
@wottpal
Copy link
Member

wottpal commented Feb 5, 2024

Merged 🔀 Will issue a new release by EOD :)

Would have been nice to add some testcases for tese cases to show how they are handled. I started yesterday on creating a pr for basic vitest setup, but seems we need to adapt the Provider a bit to be able to pass along own wallets to pass along a mocked version for testing.
https://github.com/peetzweg/use-inkathon/blob/pz/vitest-setup/src/provider.test.tsx

Happy to see this. Don't have much vitest experience on my own, though!

@wottpal
Copy link
Member

wottpal commented Feb 5, 2024

Update: Had to revert to the previous behavior in 0.8.1 when loading registered deployments, as this broke ink!athon completely. Happy to revisit this at a later point though. :)

Please make sure to always test PRs here with ink!athon (somehow assumed you did). You can find info about how to do this at the very bottom of the README (How to import a development version of this package locally?).

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.

2 participants