Skip to content

fix: await Redis EXISTS call in RedisAdapter.hasKey()#684

Open
Priyanshubhartistm wants to merge 1 commit into
cameri:mainfrom
Priyanshubhartistm:fix/redis-haskey-missing-await
Open

fix: await Redis EXISTS call in RedisAdapter.hasKey()#684
Priyanshubhartistm wants to merge 1 commit into
cameri:mainfrom
Priyanshubhartistm:fix/redis-haskey-missing-await

Conversation

@Priyanshubhartistm

Copy link
Copy Markdown
Collaborator

Description

RedisAdapter.hasKey() called this.client.exists(key) without await and wrapped the result in Boolean(). Since client.exists() returns a Promise<number>, Boolean(promise) is always true (a non-null object is truthy) regardless of whether the key actually exists in Redis. Fixed by awaiting the call and comparing the resolved count to 0.

Additionally, the existing unit tests for hasKey stubbed client.exists with sinon's .returns() (a plain synchronous value) rather than .resolves() (an actual Promise, matching the real Redis client's behavior). This masked the bug entirely — both tests passed against the broken implementation purely because Boolean(1) and Boolean(0) happened to match the expected true/false values. Updated both stubs to .resolves() so they exercise the real async path.

Related Issue

#683

Motivation and Context

hasKey() always returning true means any caller using it as a cache-presence check would incorrectly treat every key as present. The issue notes this previously forced a full Postgres lookup on every event (the call site had to be commented out because the method didn't work). While the codebase has since moved that particular admission-cache check to getKey() instead, hasKey() remains a public method on ICacheAdapter and should behave correctly for any current or future caller.

How Has This Been Tested?

  • Fixed the two existing hasKey unit tests to stub client.exists with .resolves() instead of .returns(), and verified (by temporarily reverting the source fix) that the corrected test now fails against the old buggy code with AssertionError: expected true to be false — proving it actually catches this regression.
  • Ran the full unit suite: pnpm run test:unit — 1393 passing.
  • Ran the CLI test suite: pnpm run test:cli — 73 passing.
  • Ran unit coverage: pnpm run cover:unit — passes.
  • Ran pnpm lint, pnpm exec biome format (on the touched files), pnpm check:deps,
    pnpm run build:check, pnpm run build, and `pnpm run verify:cli:bu
  • Did not run the full Docker integration suite: confirmed hasKey() is not called anywhere in
    current production code paths (the admission-cache check in event-me getKey()` instead), so this change has no reachable integration surface.

Screenshots (if appropriate):

N/A — backend-only change, no UI impact.

Types of changes

  • Non-functional change (docs, style, minor refactor)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functio

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my code changes.
  • I added a changeset, or this is docs-only and I added an empty ch
  • All new and existing tests passed.

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>
@changeset-bot

changeset-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 32fafc1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
nostream Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Status

Coverage is 67.674%Priyanshubhartistm:fix/redis-haskey-missing-await into cameri:main. No base build found for cameri:main.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants