Improve CLI error handling for contract invocation arguments #2235
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #2182 - Enhanced error messages with context and suggestions for better developer experience when invoking contracts, especially with bash scripts. - Add descriptive error messages with expected vs received values - Pre-validate JSON arguments to catch errors early - Detect common mistakes (malformed JSON, type mismatches, invalid addresses) - Include actionable suggestions and best practices documentation - Add comprehensive tests (5 new unit tests, all 33 tests passing) Example improvement: Before: Error: parsing argument amount: InvalidValue(Some(U64)) After: Failed to parse argument 'amount' - Expected u64 but received '"100"' Suggestion: For numbers, ensure no quotes around the value
…organization
Resolves three clippy pedantic lints in contract argument parsing:
- **too_many_lines**: Refactored the 114-line `build_host_function_parameters_with_filter`
function by extracting logical components into focused helper functions:
- `build_clap_command()`: Handles clap command setup and subcommand registration
- `parse_command_matches()`: Manages command line parsing and help message handling
- `get_function_spec()`: Retrieves and validates function specifications
- `parse_function_arguments()`: Orchestrates argument parsing for all function inputs
- `parse_single_argument()`: Handles individual argument parsing with type validation
- `parse_file_argument()`: Manages file-based argument loading (JSON and binary)
- `build_invoke_contract_args()`: Constructs final InvokeContractArgs structure
- **map_unwrap_or**: Replaced `map().unwrap_or_else()` pattern with explicit `match`
statement in `get_available_functions()` for better error handling clarity
- **uninlined_format_args**: Updated test assertion to use inline format arguments
(`{case}` instead of `{}, case`) for improved readability
The refactoring maintains identical functionality while improving code maintainability,
readability, and adherence to Rust best practices. Each extracted function has a
single responsibility and clear ownership semantics.
All existing tests pass and clippy checks are clean.
fnando
reviewed
Oct 8, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR enhances error handling for contract invocation arguments in the Stellar CLI, providing more detailed and actionable error messages for developers invoking contracts, especially when using bash scripts. The changes improve argument parsing with better context, type validation, and specific suggestions for common mistakes.
Key changes:
- Enhanced error messages with context including expected vs. received types and actionable suggestions
- Pre-validation of JSON arguments to catch malformed input early
- Refactored argument parsing into focused helper functions for better maintainability
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/soroban-cli/src/commands/contract/arg_parsing.rs | Major refactoring of error types and argument parsing logic with enhanced error messages and modular helper functions |
| cmd/crates/soroban-test/tests/it/help.rs | Updated test to match new structured error format for missing arguments |
| Contract_Invocation_Best_Practices.md | New documentation providing best practices for contract invocation with bash scripts and JSON handling |
… feedback Addresses all feedback from GitHub PR review #2235: **Error Handling Improvements:** - Replace panic-prone `unwrap()` with proper error handling in `parse_single_argument()` - Add context-aware error suggestions based on expected type and received value - Enhance `CannotParseArg` error with dynamic `suggestion` field **Code Organization:** - Extract `is_primitive_type()` helper function to improve readability - Add `get_context_suggestions()` for intelligent error guidance - Maintain necessary `mut` keyword where required by API **Context-Aware Error Messages:** - Numbers with quotes: "For numbers, ensure no quotes around the value (e.g., use 100 instead of \"100\")" - Decimal for integers: "Integer types don't support decimal values - use a whole number" - Invalid booleans: "For booleans, use 'true' or 'false' (without quotes)" - Invalid addresses: "For addresses, use format: G... (account), C... (contract), or identity name" - Unquoted strings: "For strings, ensure the value is properly quoted" - Complex types: Type-specific formatting guidance **Testing:** - Tested all 13 error message types - Verify context-aware suggestions work correctly - Ensure all existing functionality remains intact **Developer Experience:** - Transform cryptic errors like "InvalidValue(Some(U64))" into actionable guidance - Provide specific examples and alternative approaches in error messages - Include troubleshooting steps for common issues (file access, JSON syntax, etc.)
- Update println! statements to use inline format arguments - Maintain test functionality while adhering to clippy pedantic rules - All tests continue to pass (7/7 argument parsing tests)
- Fix prettier formatting issues in Contract_Invocation_Best_Practices.md - Ensures CI checks pass for markdown formatting - All markdown files now comply with prettier rules
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2182 - Enhanced error messages with context and suggestions for better developer experience when invoking contracts, especially with bash scripts.
Providing detailed, context-rich error messages for contract invocation arguments, including expected vs. received types and actionable suggestions.
Example improvement:
Before: Error: parsing argument amount: InvalidValue(Some(U64))
After: Failed to parse argument 'amount' - Expected u64 but received '"100"'
Suggestion: For numbers, ensure no quotes around the value