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

Improve error handling when imported smart contract is not found #329

Closed
mitschabaude opened this issue Dec 2, 2022 · 2 comments · Fixed by #586
Closed

Improve error handling when imported smart contract is not found #329

mitschabaude opened this issue Dec 2, 2022 · 2 comments · Fixed by #586

Comments

@mitschabaude
Copy link
Contributor

When importing a smart contract throws an error, the CLI assumes that it "failed to find the smart contract in your build directory". It logs that error message to the user and exits.

zkapp-cli/src/lib/deploy.js

Lines 222 to 228 in 35f48b1

smartContractImports = await import(smartContractImportPath);
} catch (_) {
log(
red(
` Failed to find the "${contractName}" smart contract in your build directory.\n Please confirm that your config.json contains the name of the smart contract that you desire to deploy to this deploy alias.`
)
);

However, there are many errors that can happen when importing a contract. Thererfore, it's a bad idea to swallow the error like that - it makes it extremely hard for users to find the real error. See #328 for example

Proposed solution is to specifically test whether we can find the contract file in the build directory, and log the existing error if we don't. After that, we just try to import the smart contract. If that fails, node will throw the proper error, which enables the user to identify the problem. If we want to print something pretty, or change the error color, we can achieve that with a try .. catch, but not at the cost of removing the stack trace / full error message.

@nicc nicc added the to-discuss label Jan 3, 2023
@jasongitmail
Copy link
Contributor

Discussed with Yoni during review. +1 for this

@jasongitmail jasongitmail changed the title Don't hide errors when importing smart contract Improve error handling when imported smart contract is not found Jan 5, 2023
@xendarboh
Copy link

This can eat space time. console.log(_)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants