fix(staking-cli): improve metadata fetching robustness and error messages#3933
fix(staking-cli): improve metadata fetching robustness and error messages#3933
Conversation
…ages Fixes metadata fetching to handle edge cases and provide clear error messages when URLs return incorrect or malformed content. Changes: - Ignore Content-Type headers completely (GitHub raw URLs serve JSON as text/plain) - Parse JSON in two steps to distinguish syntax errors from schema errors - Show clear schema-only errors for valid JSON with wrong structure - Preserve dual-format errors for truly malformed content - Add empty body detection with clear error message - Refactor test infrastructure with MetadataServerBuilder pattern - Improve MetadataServer API ergonomics (add_cli_args instead of cli_args) Error message improvements: Before: "failed to parse as JSON (missing field) or OpenMetrics (...)" After: "valid JSON but incorrect schema: missing field `pub_key`" This fixes validator registration with GitHub-hosted metadata files while maintaining backward compatibility with all metadata sources.
|
/gemini review |
Summary of ChangesHello @sveitser, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness of metadata fetching by ignoring Content-Type headers and attempting to parse content as JSON first, then falling back to OpenMetrics. This handles edge cases like GitHub raw URLs serving JSON as text/plain. The error messaging is also greatly improved by distinguishing between JSON syntax errors and schema errors, and by providing combined error messages when both formats fail to parse. The test infrastructure has been substantially refactored using a builder pattern, which enhances test clarity and maintainability. A comprehensive set of new tests has been added to validate the new logic and error handling. The changes are of high quality and well-executed. I have one suggestion to strengthen a test assertion.
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness of metadata fetching by ignoring Content-Type headers, implementing a two-step JSON parsing process for better error diagnostics, and adding checks for empty response bodies. The refactoring of the test infrastructure to use a MetadataServerBuilder pattern is a great enhancement, making the tests cleaner and more maintainable. The new tests are comprehensive and effectively validate the new logic and error handling. Overall, this is a high-quality contribution.
Replace string-based error handling with typed MetadataError enum. URL context is added by callers via .with_context() following idiomatic Rust error handling patterns. Changes: - Add MetadataError enum (SchemaError, BothFormatsFailed, EmptyBody, FetchError) - Callers add URL context: preview-metadata, validate_metadata_uri() - Replace string assertions with assert_matches! for type safety - Add integration tests verifying users see URLs in error output - Document when each error variant occurs Error messages now show which URL failed: "from http://example.com/metadata: valid JSON but incorrect schema: missing field `pub_key`" Makes production debugging easier for validators while maintaining clean, idiomatic error handling.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness of metadata fetching and provides much clearer error messages, which is a great enhancement for user experience and debugging. The introduction of the MetadataError enum for structured error handling is a solid improvement. The logic to ignore Content-Type and perform a two-step JSON parsing to distinguish between syntax and schema errors is well-implemented and addresses real-world issues like GitHub raw URLs.
The refactoring of the test suite is also a major highlight. The MetadataServerBuilder pattern makes test setup cleaner and more maintainable. The new tests are comprehensive, covering various success and failure scenarios, including the new error conditions and edge cases like empty responses.
Overall, this is a high-quality contribution that improves both the application logic and its testability. The code is clean, well-documented, and thoroughly tested.
Fixes metadata fetching to handle edge cases and provide clear error messages when URLs return incorrect or malformed content.
Changes:
Error message improvements:
Before: "failed to parse as JSON (missing field) or OpenMetrics (...)" After: "valid JSON but incorrect schema: missing field
pub_key"This fixes validator registration with GitHub-hosted metadata files while maintaining backward compatibility with all metadata sources.