Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: improve uninformative error messages #10201

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

GarmashAlex
Copy link

Motivation

Error messages in Chisel's dispatcher were uninformative in some cases, making it difficult for users to understand what went wrong and how to fix the issue. Simple messages like "Failed to load session!" or "Invalid fork URL!" didn't provide enough context for users to troubleshoot effectively.

Solution

Improved several error messages in the Chisel dispatcher by adding more context and guidance to make them more helpful and actionable. The changes maintain the existing code logic and message format while providing clearer details about what went wrong and potential fixes.

For example:

  • "Too many arguments supplied!" → "Too many arguments supplied! Please check command syntax."
  • "Failed to load session!" → "Failed to load session! The session ID may not exist or might be corrupted."
  • "Invalid fork URL!" → "Invalid fork URL! Please provide a valid RPC endpoint URL."
  • "No command supplied!" → "No command supplied! Please provide a valid command after '!'."

These enhanced error messages will help users understand issues more quickly without having to dig through the code.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@@ -258,7 +258,7 @@ impl ChiselDispatcher {
self.session.id.as_ref().unwrap()
)))
} else {
DispatchResult::CommandFailed(Self::make_error("Too many arguments supplied!"))
DispatchResult::CommandFailed(Self::make_error("Too many arguments supplied! Please check command syntax."))
Copy link
Member

Choose a reason for hiding this comment

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

I think here it would be better to include all the arguments so the user can easily double check it

Copy link
Author

Choose a reason for hiding this comment

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

@mattsse Corrected. Thank you for looking this through

@GarmashAlex GarmashAlex requested a review from mattsse March 29, 2025 19:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants