chore: improve serverless refresh error#4436
Conversation
|
🚅 Deployed to the rivet-pr-4436 environment in rivet-frontend
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
fc4e0a5 to
b4a3ace
Compare
92af256 to
ed101ee
Compare
|
PR Review: chore: improve serverless refresh error. Small improvement promoting anyhow! to a typed RivetError. Bug: two-arg error macro treats second arg as static string, so the reason placeholder appears verbatim in default_message - needs a third arg for dynamic interpolation. Minor: ServerlessRunnerPool lacks Clone unlike Actor and Runner in the same file. Positives: removing unused anyhow import, cleaner await map_err chain, structured meta payload. |
|
PR Review: chore: improve serverless refresh error. Overall: Approved. Small, well-scoped improvement. Summary: (1) engine/packages/pegboard/src/errors.rs adds a new FailedToFetchMetadata variant to ServerlessRunnerPool wrapping ServerlessMetadataError. (2) engine/packages/api-public/src/runner_configs/utils.rs replaces a generic anyhow! string error with the typed ServerlessRunnerPool::FailedToFetchMetadata error, removes the unused anyhow::anyhow import, and renames result to metadata for clarity. What is good: Replacing an untyped anyhow! string with a structured RivetError variant is the correct pattern per conventions. The unused import is cleaned up. The variable rename improves readability. ServerlessMetadataError already derives Serialize, Deserialize, Clone, and Debug satisfying trait bounds for embedding in a RivetError variant. Minor observation: The new error uses debug formatting {reason:?} in the message template. ServerlessMetadataError does not implement Display so this produces verbose Rust debug output. Not a bug, but if user-facing consider implementing Display for ServerlessMetadataError and switching to {reason}. Asymmetry note: fetch_serverless_runner_metadata maps all ServerlessMetadataError variants to RequestFailed discarding the original variant, while refresh_runner_config_metadata preserves the full structured error. Appears intentional but worth confirming. No convention issues: correct use of derive(RivetError), no rivet.gg domains, no logging violations, no new raw anyhow! macros. No blocking issues found. |
Merge activity
|
b4a3ace to
59396a4
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: