Skip to content

validate --data-dir is not a file before startup#2304

Open
evanbranigan-dotcom wants to merge 1 commit intozingolabs:devfrom
evanbranigan-dotcom:fix-wallet-dir-error-message
Open

validate --data-dir is not a file before startup#2304
evanbranigan-dotcom wants to merge 1 commit intozingolabs:devfrom
evanbranigan-dotcom:fix-wallet-dir-error-message

Conversation

@evanbranigan-dotcom
Copy link
Copy Markdown

Fixes: #2272

When --data-dir points to a wallet file (e.g. --data-dir /path/to/zingo-wallet.dat) instead of its parent directory, zingolib panics with:

panic!("Couldn't create zcash directory!")

This gives no indication that the path should be a directory, not a file.

Changes

  • Add early validation in ConfigTemplate::fill() that checks if the provided --data-dir path is an existing file
  • Return a clear error message with the correct parent directory as a hint:
    --data-dir must be a directory, not a file.
    You provided: /path/to/zingo-wallet.dat
    Hint: use the parent directory instead, e.g. --data-dir /path/to
    
  • Add test data_dir_is_a_file in the error_cases module
  • All 71 existing tests pass

AI disclosure: Fix implemented with Claude Code (Anthropic), reviewed and verified by human contributor.

When --data-dir points to a wallet file instead of its parent
directory, zingolib panics with "Couldn't create zcash directory!"
which gives the user no indication of what went wrong.

Add early validation in ConfigTemplate::fill() that checks if
the provided path is an existing file and returns a clear error
message with a hint to use the parent directory instead.

AI disclosure: fix implemented with Claude Code (Anthropic),
reviewed and verified by human contributor.

Fixes: zingolabs#2272
Copy link
Copy Markdown
Contributor

@Oscar-Pepper Oscar-Pepper left a comment

Choose a reason for hiding this comment

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

great work! the formatting seems incorrect during testing, the "\n" newline characters are not working. could we fix this?

example:
$ ./target/release/zingo-cli --data-dir ~/wallets/test_5/zingo-wallet.dat
Error filling config template: "--data-dir must be a directory, not a file.\nYou provided: /home/oscar/wallets/test_5/zingo-wallet.dat\nHint: use the parent directory instead, e.g. --data-dir /home/oscar/wallets/test_5"

Copy link
Copy Markdown
Author

@evanbranigan-dotcom evanbranigan-dotcom left a comment

Choose a reason for hiding this comment

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

Thanks for catching this! The newlines in the error string are correct — the issue is in run_cli() at line 623, which uses {e:?} (Debug formatting) to print the error. Debug wraps the string in quotes and escapes \n as literal characters.

Changing {e:?} to {e} fixes it — and would also fix the existing server URI error on line 431 which has the same \n pattern.

Since that's pre-existing code outside the scope of this PR, would you prefer I fix it here as a second commit, or should I open a separate issue for it?

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.

Zingo-CLI does not provide intuitive error message when wallet file dir is incorrect.

2 participants