Emit ApiErrorOccurred metric for request timed out error - #932
Conversation
WalkthroughThe change introduces a new error code constant ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
||
| // handle possible request timeouts | ||
| if message.Error != nil && message.Error.Code == errCodeTimeout { | ||
| w.metrics.ApiErrorOccurred() |
There was a problem hiding this comment.
should we be emitting this metric any time message.Error != nil?
There was a problem hiding this comment.
This is generally handled from the handleError function: https://github.com/onflow/flow-evm-gateway/blob/main/api/utils.go#L140-L142.
There are plenty of cases where message.Error != nil and the actual error is simply a user error, e.g.
{
"jsonrpc": "2.0",
"id": 1,
"error": {
"code": -32602,
"message": "too many arguments, want at most 0"
}
}Emitting the ApiErrorOccurred metric for such cases, would only cause false alerts.
We are only interested in the special case where message.Error.Code == errCodeTimeout, because in this case, the handleError function never gets called.
Closes: #931
Description
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.