Treat empty Hive-Engine rewards response as no rewards, not an error#55
Conversation
An account with no Hive-Engine activity gets an empty object back from the rewards upstream. Both that and a non-object response threw into the catch, which logged an error and returned an empty array. The empty result was correct; the error log was not, and it fired on every such request. Return the empty array directly instead. Genuine failures still reach the catch and are still logged. Responses are unchanged.
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR changes how Hive-Engine rewards responses are handled.
Confidence Score: 4/5This is close, but one rewards error path should be fixed before merging.
dotnet/EcencyApi/Handlers/WalletApi.Engine.cs Important Files Changed
Reviews (2): Last reviewed commit: "Keep logging non-object rewards payloads" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A non-object body means the rewards upstream is degraded (unparseable body, or valid JSON that is not an object), which is worth a log line. Only the empty object - the normal answer for an account with no Hive-Engine activity - is silent now.
| if (response.Json is not JsonObject obj) | ||
| throw new Exception("No rewards data returned"); | ||
|
|
||
| var rawValues = obj.Select(kv => kv.Value).ToList(); | ||
| if (rawValues.Count == 0) throw new Exception("No rewards data returned"); | ||
| throw new Exception("Unexpected rewards payload"); |
There was a problem hiding this comment.
This check only sends non-object payloads through the catch path. If the rewards service returns an object-shaped error body like { "error": "rate limit" } or { "message": "Internal Server Error" }, it passes this branch, the loop feeds the string value into HiveEngine.ConvertRewardsStatus, and pendingToken stays null so the result is filtered down to an empty rewards array. Callers then receive “no rewards” with no error log, even though the upstream response was degraded. Please validate the expected reward-entry shape, or the upstream status, while keeping only an actually empty object as the no-rewards case.
There was a problem hiding this comment.
This one is pre-existing behavior, not something this PR introduces — main does exactly the same thing with an object-shaped error body.
main's only guard was rawValues.Count == 0. A body like {"error": "rate limit"} has one key, so it never hit that throw either: it went through the same loop, ConvertRewardsStatus returned nulls (JsVal.Prop on a non-object yields null, so pendingToken is null and pendingRewards is NaN — no exception), the pendingToken is > 0 filter dropped it, and the caller got an empty array with no log. Identical before and after.
The only guard this PR removes is the Count == 0 one, i.e. the genuinely-empty object. The path you're describing is untouched by the diff, so there is nothing here to regress.
Worth saying the underlying observation is fair: an object-shaped error body does read as "no rewards". But that has always been true, and closing it means validating the reward-entry shape or the upstream status — a behavior change beyond a log-noise fix, and one that needs to be weighed against the response-parity contract this service maintains. Leaving it out of scope here rather than smuggling it in.
1 participant
FetchEngineRewardstreated "this account has no Hive-Engine rewards" as a failure.The rewards upstream answers with an empty object for an account with no Hive-Engine activity. The handler threw on that (and on a non-object response), fell into the catch, logged
failed to get unclaimed engine rewards No rewards data returned, and returned an empty array. The empty array was the right answer — but it was reached via an exception and an error log, on every request for such an account. That is a hot-path write on the portfolio endpoints, against invariant 6.This returns the empty result directly. An empty object now falls through the existing loop to an empty array, which is what the catch produced anyway.
Not a behavior change:
Verified by running this build and
mainside by side against the same upstreams and diffing/wallet-api/portfolio-v2: responses are byte-identical for accounts with no engine rewards, and the error log goes from one line per request to none. Accounts that do have rewards are byte-identical too, apart from live chain values (HP balance, APR, staked token accrual) that move between calls. Test suite green.