Skip to content

redis-cache.ts's webhook-dedup path records no error metric, unlike its two cache siblings #8363

Description

@JSONbored

Context

src/selfhost/redis-cache.ts implements webhook-delivery-dedup helpers isWebhookDeliveryDuplicate
(lines 18-29) and rememberWebhookDelivery (lines 32-38). Both wrap their Redis calls in a catch
block that silently swallows errors — no error metric is recorded on failure.

Both of this file's Redis-cache siblings record an error metric on the exact same failure mode:

  • src/selfhost/redis-token-cache.ts calls recordTokenCacheMetric("error") in both get() and
    set()'s catch blocks.
  • src/selfhost/redis-response-cache.ts calls recordRedisResponseCacheMetric("error") in both
    get() and set()'s catch blocks.

redis-cache.ts already imports incr and uses it for the dedup-hit case, but has no equivalent
metric call for the error case. As a result, a sustained Redis outage affecting webhook dedup is
completely invisible in metrics/dashboards, unlike the identical failure mode in either sibling cache.

Existing test confirms the current (gap) behavior is deliberate-looking but unmetriced:
test/unit/selfhost-redis-cache.test.ts:124 — "returns false without incrementing when Redis get
throws" — codifies today's no-metric-on-error behavior; this is the test to update once the metric
call is added.

Requirements

  • Add an error-result metric call in both isWebhookDeliveryDuplicate's and
    rememberWebhookDelivery's catch blocks in src/selfhost/redis-cache.ts, mirroring the exact
    pattern redis-token-cache.ts/redis-response-cache.ts already use (a named metric-recording
    function called with an "error" result, not a bare incr call — check the actual current metric
    function signature in this file/its imports before writing the patch).
  • Do not change the existing fail-open behavior (returning false/no-op on error) — this issue is
    scoped to adding observability, not changing the error-handling contract.
  • Do not touch redis-token-cache.ts or redis-response-cache.ts — they are already correct and are
    the reference pattern to match.

Deliverables

  • isWebhookDeliveryDuplicate and rememberWebhookDelivery in src/selfhost/redis-cache.ts both
    record an error metric when their Redis call throws, matching the sibling caches' pattern.
  • test/unit/selfhost-redis-cache.test.ts:124's existing test ("returns false without incrementing
    when Redis get throws") is updated to assert the new error-metric call fires, alongside the
    existing fail-open-return assertion.

Test Coverage Requirements

This repo's Codecov patch gate is 99%+ of changed lines and branches (src/** is covered). The new
metric-call lines in both catch blocks must be exercised by the updated test above (and an equivalent
new/updated test for rememberWebhookDelivery's catch block if one doesn't already exist).

Expected Outcome

A sustained Redis outage affecting webhook-delivery dedup is now visible in the same metric surface
that already catches token-cache and response-cache Redis outages — no more blind spot specific to
this one cache.

Links & Resources

  • src/selfhost/redis-cache.ts:18-38 — the two functions to fix
  • src/selfhost/redis-token-cache.ts — reference pattern (error metric in get/set)
  • src/selfhost/redis-response-cache.ts — reference pattern (error metric in get/set)
  • test/unit/selfhost-redis-cache.test.ts:124 — the existing test to update

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions