Skip to content

fix: recover metering events after delivery failures - #1639

Merged
kang-heewon merged 1 commit into
trunkfrom
fix/1574-recover-metering-events
Jul 30, 2026
Merged

fix: recover metering events after delivery failures#1639
kang-heewon merged 1 commit into
trunkfrom
fix/1574-recover-metering-events

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Jul 30, 2026

Copy link
Copy Markdown
Member

Outcome

  • Keeps usage persistence and event publication as explicit recoverable stages so publication failures can resume without recording usage twice.
  • Preserves stable logical event identities and original quota results across retries while coordinating with legacy idempotency keys during rolling deploys.
  • Makes storage replay and Redis multi-key script capabilities explicit, with runnable in-memory adapters and smoke coverage.

Verification

  • pnpm test
  • pnpm typecheck
  • pnpm lint
  • pnpm check
  • pnpm public-api:check
  • pnpm docs:api:check
  • pnpm quick-start-lambda:smoke
  • pnpm create-croco-app:smoke -- goal-saas-api

Review notes

The metering delivery scripts intentionally require multi-key Redis script support. Redis Cluster deployments that cannot keep these keys in one script execution remain unsupported and are documented explicitly.

Fixes #1574.

Summary by CodeRabbit

  • 새로운 기능
    • 미터링 재시도 시 동일 이벤트 식별자/처리 단계 및 사용량 결과를 복원해 중복 기록을 방지합니다.
    • 대기 중 이벤트 퍼블리싱과 처리 완료/중단을 단계적으로 관리합니다.
  • 버그 수정
    • 이벤트 퍼블리싱 실패 후에도 안전하게 재개되며, 쿼터 초과 결과의 일관성이 유지됩니다.
  • 문서
    • 재시도 계약(멱등성), 이벤트 ID 생성 규칙(선택 파라미터 포함), Redis multi-key 요구사항 및 관련 API 문서를 보강했습니다.
  • 테스트
    • 재청구/재플레이 시나리오와 중복 방지 동작을 추가 검증했습니다.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kang-heewon, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7c45416c-a7eb-48c6-b614-9a7998aa7c1d

📥 Commits

Reviewing files that changed from the base of the PR and between e82af59 and 607041e.

⛔ Files ignored due to path filters (1)
  • packages/problems-core/src/generated/problem-code-registry.ts is excluded by !**/generated/**
📒 Files selected for processing (45)
  • .changeset/calm-geckos-recover.md
  • docs/problem-code-registry.json
  • examples/quick-start-lambda/src/integrations/inMemoryMetering.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/inMemoryAdapters.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/tests/SaasDemo.spec.ts
  • packages/docs/src/content/docs/api/events-core/src/classes/DomainEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/IdempotencyManager.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/MeteringTransitionProblem.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/QuotaExceededEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/RedisUsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/UsageRecordedEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/RedisClient.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/UsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/MeteringProcessingClaim.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/PendingMeteringDelivery.md
  • packages/docs/src/content/docs/api/metering-upstash/src/classes/UpstashRedisClient.md
  • packages/docs/src/content/docs/api/problems-core/src/classes/Problem.md
  • packages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.md
  • packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
  • packages/events-core/src/libs/DomainEvent.ts
  • packages/events-core/src/tests/DomainEvents.spec.ts
  • packages/metering-core/README.md
  • packages/metering-core/src/index.ts
  • packages/metering-core/src/libs/IdempotencyManager.ts
  • packages/metering-core/src/libs/MeteringService.ts
  • packages/metering-core/src/libs/QuotaManager.ts
  • packages/metering-core/src/libs/RedisClient.ts
  • packages/metering-core/src/libs/RedisUsageStorage.ts
  • packages/metering-core/src/libs/UsageStorage.ts
  • packages/metering-core/src/libs/events/QuotaExceededEvent.ts
  • packages/metering-core/src/libs/events/UsageRecordedEvent.ts
  • packages/metering-core/src/libs/events/eventIdentity.ts
  • packages/metering-core/src/libs/problems/MeteringTransitionProblem.ts
  • packages/metering-core/src/tests/IdempotencyManager.spec.ts
  • packages/metering-core/src/tests/MeteringService.spec.ts
  • packages/metering-core/src/tests/QuotaManager.spec.ts
  • packages/metering-core/src/tests/RedisMetering.integration.spec.ts
  • packages/metering-core/src/tests/RedisUsageStorage.spec.ts
  • packages/metering-core/src/tests/UsageAggregator.spec.ts
  • packages/metering-core/src/tests/UsageStorage.spec.ts
  • packages/metering-core/src/tests/events/Events.spec.ts
  • packages/metering-core/src/tests/problems/Problems.spec.ts
  • packages/metering-upstash/src/libs/UpstashRedisClient.ts
  • public-api-surface.snapshot.json
  • scripts/quick-start-lambda-smoke.mts
📝 Walkthrough

Walkthrough

메터링 처리가 delivery 상태 머신과 멱등성 계약을 사용하도록 변경되었습니다. 이벤트 ID는 결정적으로 생성되며, Redis 사용량 저장소와 인메모리 어댑터는 quota 결과와 delivery를 재시도 시 복원합니다. 관련 공개 API, 문서, 테스트 및 smoke 검증도 갱신되었습니다.

Changes

메터링 재시도 및 이벤트 계약

Layer / File(s) Summary
이벤트 아이덴티티와 공개 계약
packages/events-core/..., packages/metering-core/src/libs/events/*, packages/metering-core/src/libs/UsageStorage.ts, packages/metering-core/src/libs/RedisClient.ts, packages/metering-core/src/index.ts
이벤트 ID 생성과 명시적 이벤트 ID 전달을 추가하고, UsageStorage.replayContractRedisClient.scriptKeyAccess 계약과 관련 공개 타입을 확장했습니다.
메터링 delivery 상태 머신
packages/metering-core/src/libs/IdempotencyManager.ts, packages/metering-core/src/libs/MeteringService.ts
처리 claim, publishing, events pending, 완료 및 중단 단계를 Redis Lua 전환으로 관리하며, 저장 후 발행 실패를 재시도할 수 있도록 처리 흐름을 변경했습니다.
쿼터 결과와 저장 멱등성
packages/metering-core/src/libs/RedisUsageStorage.ts, packages/metering-core/src/tests/RedisUsageStorage.spec.ts
동일 idempotency 키의 quota 결과를 저장·복원하고, 재시도 시 사용량 재집계와 중복 기록을 방지하도록 변경했습니다.
인메모리 어댑터와 검증
examples/quick-start-lambda/..., packages/create-croco-app/templates/saas/..., scripts/quick-start-lambda-smoke.mts
인메모리 Redis delivery 상태 머신을 추가하고, replay 테스트와 빌드·런타임 smoke 검증을 연결했습니다.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning 메터링 수정과 무관한 credits-core/problems-core 문서 변경이 포함되어 있어 범위를 벗어난 수정이 있습니다. 해당 문서 링크 변경을 제거하거나, 메터링 회복 작업과 직접 연결되는 경우로만 남기세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목이 메터링 이벤트 복구라는 핵심 변경을 간결하게 요약합니다.
Linked Issues check ✅ Passed 사용량 저장과 이벤트 발행을 분리된 재개 가능 단계로 바꾸고, 중복 기록 방지 및 양 경로 회귀 테스트를 추가해 #1574 요구를 충족합니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1574-recover-metering-events

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kang-heewon
kang-heewon force-pushed the fix/1574-recover-metering-events branch from efdfcd0 to 28e369b Compare July 30, 2026 13:08
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

📊 Benchmark Results

✅ All benchmarks passed

Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 9.2μs 30.0ms 8.2μs +13.0% -
CrocoApp lambdaHandler (10 controllers) 283.5μs 50.0ms 258.4μs +9.7% -
Lambda cold-start simulation 428.2μs 80.0ms 418.1μs +2.4% -
Lambda cold-start with headers 394.6μs 80.0ms 369.7μs +6.7% -
Lambda cold-start with binary body 372.3μs 80.0ms 339.1μs +9.8% -
Lambda cold-start with query params 317.6μs 80.0ms 301.3μs +5.4% -
Lambda cold-start with authorizer context 315.2μs 80.0ms 299.8μs +5.1% -
Lambda cold-start realistic scenario 314.7μs 80.0ms 299.2μs +5.2% -
EventBusConfig.start (10 handlers) 1.7μs 10.0ms 1.4μs +16.1% -
EventPublisher.publishNow single event 1.9μs 2.0ms 1.7μs +10.1% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.1μs +0.0% -
Container.get singleton (cold) 79.4μs 5.0ms 70.3μs +13.0% -
Container.register × 50 components 3.5ms 10.0ms 3.2ms +9.6% -
Container.validate (50 components) 4.1ms 20.0ms 3.4ms +19.9% -
Container.get singleton (warm) 1.7μs 500.0μs 1.6μs +1.8% -
TelemetryRuntime.init (lambda preset) 2.3μs 200.0ms 1.1ms -99.8% -
lambdaPreset config creation 1.5μs 2.0ms 1.4μs +2.2% -

Updated: 2026-07-30T18:33:38.745Z · Commit: e4484d2

@kang-heewon
kang-heewon force-pushed the fix/1574-recover-metering-events branch 2 times, most recently from 1356fe1 to 8ee1f1b Compare July 30, 2026 14:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@examples/quick-start-lambda/src/integrations/inMemoryMetering.ts`:
- Around line 108-158: Replace the unconditional fallback in
InMemoryRedisClient.eval() with an explicit throw that includes a stable
diagnostic message identifying the unsupported script, rather than returning
[1]. Apply the same fallback change in
examples/quick-start-lambda/src/integrations/inMemoryMetering.ts lines 108-158
and
packages/create-croco-app/templates/saas/apps/api-server/src/inMemoryAdapters.ts
lines 350-393; preserve all existing recognized-script branches.
- Around line 20-29: Separate the key sets used for usage-record deduplication
and idempotency reservation. In
examples/quick-start-lambda/src/integrations/inMemoryMetering.ts lines 20-29,
update record() to use a dedicated record-dedupe Set while leaving
isIdempotent() on its reservation Set; apply the same change in
packages/create-croco-app/templates/saas/apps/api-server/src/inMemoryAdapters.ts
lines 209-214 so record(), including calls from checkAndRecordWithinQuota(),
uses the dedicated Set.

In
`@packages/create-croco-app/templates/saas/apps/api-server/src/tests/SaasDemo.spec.ts`:
- Around line 73-75: Update the asynchronous rejection assertion for
manager.claimMeteringProcessingOrThrow in SaasDemo.spec.ts to also use
rejects.toThrow, while preserving the existing metering/duplicate-record code
check so the test verifies both the thrown error type and its code.

In
`@packages/docs/src/content/docs/api/metering-core/src/classes/QuotaExceededEvent.md`:
- Around line 17-25: Update the QuotaExceededEvent JSDoc `@example` in
QuotaExceededEvent.ts to instantiate the event using positional arguments
matching its constructor signature (tenantId, meterId, currentUsage, quota, with
optional idempotencyKey and operationId), and remove the unsupported timestamp
field. Regenerate the TypeDoc output rather than editing the generated Markdown
directly, then verify with pnpm docs:api:check.

In
`@packages/docs/src/content/docs/api/metering-core/src/classes/RedisUsageStorage.md`:
- Around line 53-54: Update the original TypeScript JSDoc or contract text that
generates RedisUsageStorage.md so the idempotency statement explicitly scopes
deduplication and quota reuse to the composite key of tenantId, meterId, and
idempotencyKey, matching record() documentation. Regenerate the TypeDoc output
instead of editing the generated Markdown directly.

In
`@packages/docs/src/content/docs/api/metering-core/src/classes/UsageRecordedEvent.md`:
- Around line 17-25: Update the UsageRecordedEvent JSDoc `@example` and generated
UsageRecordedEvent.md example to call the positional constructor signature
(tenantId, meterId, value, idempotencyKey, optional metadata, optional
operationId). Add a valid idempotencyKey, remove the unsupported recordedAt
field, and regenerate the documentation while preserving the intended metadata
example.

In `@packages/metering-core/src/libs/IdempotencyManager.ts`:
- Around line 288-306: Update both IdempotencyManager Redis scripts at
packages/metering-core/src/libs/IdempotencyManager.ts:288-306 and :347-364 so
their return values distinguish an actual state transition or deletion from a
status/token mismatch or missing record. Preserve the successful result only
when the operation occurs, have callers inspect the result and recognize
fencing, and record diagnostic evidence at the retry/Problem boundary; the
second site requires the same applied/deletion-result handling as the
PUBLISHING-to-EVENTS_PENDING transition.
- Around line 262-268: Update the delivery handling in the claim Lua script and
its corresponding retry/claim path to preserve the original JSON string from
ARGV[2] without cjson.decode or cjson.encode round-tripping. Store the raw
delivery payload in state and return that same raw string when restoring it,
while keeping state-machine fields encoded normally.

In `@packages/metering-core/src/libs/MeteringService.ts`:
- Around line 131-148: Update the catch handling in MeteringService’s metering
flow so failures after persistenceCompleted but before publishingClaimed do not
leave the PROCESSING lease blocking retries until expiry. Add the appropriate
idempotency-manager transition that immediately invalidates or expires the lease
while preserving the PROCESSING state, and keep the existing
releaseMeteringEvents and abortMeteringProcessing paths unchanged for their
respective conditions.

In `@packages/metering-core/src/libs/RedisUsageStorage.ts`:
- Around line 103-110: Move the dedupeKey Redis GET and valid recorded-result
return ahead of the ZRANGEBYSCORE query and parsing loop in the surrounding Lua
logic. Ensure valid cached quota results return immediately, while sorted-set
aggregation runs only for legacy fallback or new-record paths.
- Around line 103-110: Update the recordedResult parsing in
checkAndRecordWithinQuota so recordedUsage is accepted only when it is a valid
numeric value before returning it via the Redis Lua result. When parsing or
numeric conversion fails, use the existing legacy fallback return { 0,
currentUsage } so the response always contains both quota values and cannot
propagate undefined or NaN.

In `@packages/metering-core/src/libs/UsageStorage.ts`:
- Around line 66-70: UsageStorage의 checkAndRecordWithinQuota 계약을 선택 사항으로 두지 말고
필수 메서드로 변경하여, 모든 구현이 동일 idempotencyKey의 최초 quota 결과를 복원하도록 타입 수준에서 요구하세요. 기존
UsageStorage 호환성이 필요한 경우 해당 메서드를 필수로 포함하는 별도 quota 저장소 계약을 정의하고 quota meter 경로가
그 계약을 사용하도록 수정하세요.

In `@packages/metering-core/src/tests/events/Events.spec.ts`:
- Around line 108-118: QuotaExceededEvent 테스트가 idempotency key만 검증하고
operationId가 eventId 생성에 반영되는지 확인하지 않습니다. 기존 테스트에 동일한 idempotency key와 서로 다른
operationId로 생성한 이벤트를 추가하고 eventId가 서로 다른지 검증하세요. 동일 operationId와 키의 재시도는 기존처럼
동일한 eventId를 유지하며, eventId 형식 및 민감한 키 비노출 검증도 유지하세요.

In `@packages/metering-core/src/tests/IdempotencyManager.spec.ts`:
- Around line 440-443: Update the asynchronous rejection assertions in
IdempotencyManager.spec.ts, including the tests around
claimMeteringProcessingOrThrow, to use rejects.toThrow(DuplicateRecordProblem)
instead of rejects.toBeInstanceOf(DuplicateRecordProblem) at all identified
occurrences.

In `@packages/metering-core/src/tests/RedisUsageStorage.spec.ts`:
- Line 126: Remove the Redis Lua source-string assertion around mockRedis.eval
in RedisUsageStorage tests, including the check for "EXISTS". Retain the
behavioral assertion that verifies duplicate record operations do not call zadd.
- Around line 958-1019: Extend the RedisUsageStorage regression coverage around
checkAndRecordWithinQuota to simulate a legacy dedupe entry containing the
string "1", where the Lua script must return the fallback result {0,
currentUsage}. Verify the request is treated as successful with the current
usage and that the usage record is not written again, using the existing Redis
mock and assertions in the rejected quota replay test as a guide.
- Around line 982-985: Replace the source-text assertions in
packages/metering-core/src/tests/RedisUsageStorage.spec.ts:982-985 with
behavioral assertions that verify SET receives dedupeKey value quota:1:13 and
usageRecordCount remains 0, avoiding expect calls inside the mock that become
RedisProblem errors. At
packages/metering-core/src/tests/RedisUsageStorage.spec.ts:126, remove the
EXISTS string assertion and rely on the existing zadd-not-called assertion at
line 257.

In `@scripts/quick-start-lambda-smoke.mts`:
- Around line 217-258: Update the async probe IIFE in the “metering replay”
phase to attach a catch handler that prints the failure error and sets
process.exitCode to 1. Preserve the existing successful execution flow and probe
assertions, and ensure rejected promises are explicitly propagated to the
process status before runPhase completes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b5c4d46c-350c-44d9-9bdb-9d858e78aa19

📥 Commits

Reviewing files that changed from the base of the PR and between 5d08b1b and 8ee1f1b.

📒 Files selected for processing (37)
  • .changeset/calm-geckos-recover.md
  • examples/quick-start-lambda/src/integrations/inMemoryMetering.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/inMemoryAdapters.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/tests/SaasDemo.spec.ts
  • packages/docs/src/content/docs/api/events-core/src/classes/DomainEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/IdempotencyManager.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/QuotaExceededEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/RedisUsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/UsageRecordedEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/RedisClient.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/UsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/MeteringProcessingClaim.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/PendingMeteringDelivery.md
  • packages/docs/src/content/docs/api/metering-upstash/src/classes/UpstashRedisClient.md
  • packages/events-core/src/libs/DomainEvent.ts
  • packages/events-core/src/tests/DomainEvents.spec.ts
  • packages/metering-core/README.md
  • packages/metering-core/src/index.ts
  • packages/metering-core/src/libs/IdempotencyManager.ts
  • packages/metering-core/src/libs/MeteringService.ts
  • packages/metering-core/src/libs/RedisClient.ts
  • packages/metering-core/src/libs/RedisUsageStorage.ts
  • packages/metering-core/src/libs/UsageStorage.ts
  • packages/metering-core/src/libs/events/QuotaExceededEvent.ts
  • packages/metering-core/src/libs/events/UsageRecordedEvent.ts
  • packages/metering-core/src/libs/events/eventIdentity.ts
  • packages/metering-core/src/tests/IdempotencyManager.spec.ts
  • packages/metering-core/src/tests/MeteringService.spec.ts
  • packages/metering-core/src/tests/QuotaManager.spec.ts
  • packages/metering-core/src/tests/RedisMetering.integration.spec.ts
  • packages/metering-core/src/tests/RedisUsageStorage.spec.ts
  • packages/metering-core/src/tests/UsageAggregator.spec.ts
  • packages/metering-core/src/tests/UsageStorage.spec.ts
  • packages/metering-core/src/tests/events/Events.spec.ts
  • packages/metering-upstash/src/libs/UpstashRedisClient.ts
  • public-api-surface.snapshot.json
  • scripts/quick-start-lambda-smoke.mts

Comment thread examples/quick-start-lambda/src/integrations/inMemoryMetering.ts
Comment thread examples/quick-start-lambda/src/integrations/inMemoryMetering.ts Outdated
Comment thread packages/docs/src/content/docs/api/metering-core/src/classes/RedisUsageStorage.md Outdated
Comment thread packages/metering-core/src/tests/IdempotencyManager.spec.ts
Comment thread packages/metering-core/src/tests/RedisUsageStorage.spec.ts Outdated
Comment thread packages/metering-core/src/tests/RedisUsageStorage.spec.ts
Comment thread packages/metering-core/src/tests/RedisUsageStorage.spec.ts Outdated
Comment thread scripts/quick-start-lambda-smoke.mts
@kang-heewon
kang-heewon force-pushed the fix/1574-recover-metering-events branch from 8ee1f1b to e82af59 Compare July 30, 2026 16:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/metering-core/src/libs/RedisUsageStorage.ts (1)

97-119: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

legacy dedupe 값 폴백이 quota 초과를 항상 exceeded: false로 보고합니다.

record()/isIdempotent()(및 이전 배포 버전)가 남긴 dedupe 값은 '1'이므로 100–104행 파싱이 실패하고, 117–119행에서 무조건 { 0, currentUsage }를 반환합니다. 롤링 배포 중 첫 시도가 legacy 형식을 남긴 뒤 재시도가 들어오면 실제로 quota를 초과한 요청이 정상 처리로 보고되어 QuotaExceededProblemQuotaExceededEvent가 모두 누락됩니다. 저장된 사용량 기준으로 초과 여부를 재판정하면 legacy 경로에서도 quota 계약이 유지됩니다.

🐛 제안 수정
 if recordedResult then
-  return { 0, currentUsage }
+  local legacyExceeded = currentUsage > quota
+  return { legacyExceeded and 1 or 0, currentUsage }
 end

코딩 가이드라인의 “silent fallback으로 실패를 숨기지 말고 ... 실패와 복구 경로를 모델링한다”에 근거합니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/metering-core/src/libs/RedisUsageStorage.ts` around lines 97 - 119,
Update the legacy dedupe fallback in the Lua logic used by RedisUsageStorage so
a stored value such as "1" does not unconditionally return exceeded=false. When
recordedResult is present but the quota-formatted parse fails, recompute the
exceeded flag from currentUsage using the same quota limit and return that
result with currentUsage, preserving the existing behavior for valid
quota-formatted dedupe values.

Source: Coding guidelines

packages/metering-core/src/tests/RedisMetering.integration.spec.ts (2)

232-235: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

비동기 실패 검증에 rejects.toThrow를 추가하세요.

구조 검증은 유지하되, 동일 Promise에 rejects.toThrow()를 먼저 적용해 비동기 오류 검증 계약을 충족하세요.

  • packages/metering-core/src/tests/RedisMetering.integration.spec.ts#L232-L235: 제한 Redis client 실패 assertion에 rejects.toThrow()를 추가하세요.
  • packages/metering-core/src/tests/RedisMetering.integration.spec.ts#L261-L264: ambiguous response 실패 assertion에 동일하게 추가하세요.

As per coding guidelines, “비동기 오류는 rejects.toThrow로 검증한다.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/metering-core/src/tests/RedisMetering.integration.spec.ts` around
lines 232 - 235, Update both failure assertions in
RedisMetering.integration.spec.ts at lines 232-235 and 261-264 to first apply
rejects.toThrow() to the same Promise, while preserving the existing
rejects.toMatchObject structure checks for the error code and operation.

Source: Coding guidelines


221-225: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

재시도와 실패 시도에 동일한 Redis key prefix를 사용하세요.

keyPrefix: "" 때문에 제한 클라이언트의 실패 시도는 일반 클라이언트가 재시도·검증하는 ${connection.keyPrefix} 키와 다른 물리 키를 사용합니다. 따라서 실패 후 marker가 남아도 이 테스트가 통과할 수 있습니다.

수정안
-      keyPrefix: "",
+      keyPrefix: connection.keyPrefix,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/metering-core/src/tests/RedisMetering.integration.spec.ts` around
lines 221 - 225, Update the restrictedClient setup in the Redis metering
integration test to use the same key prefix as connection.client, specifically
connection.keyPrefix, instead of an empty prefix. Keep the retry and
verification paths unchanged so both clients target the identical physical Redis
key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/metering-core/src/libs/events/eventIdentity.ts`:
- Around line 3-13: Update createMeteringEventId so the SHA-256 input excludes
operationId and hashes only eventName, tenantId, meterId, and idempotencyKey.
Keep the existing argument and return structure otherwise unchanged, ensuring
identical idempotency inputs always produce the same event ID.

In `@packages/metering-core/src/libs/IdempotencyManager.ts`:
- Around line 242-279: Update all five metering transition methods, including
markMeteringEventsPublishing and the corresponding release/claim transition
methods, so each Redis Lua script returns a failure reason code alongside its
success flag, distinguishing missing keys, unexpected status, and token fencing
mismatches. Propagate the returned status and reason through
requireStagedTransition so transition failures retain actionable evidence about
the current state and expected token/status before throwing.
- Around line 448-452: Update requireStagedTransition and the
releaseMeteringEvents catch path so a zero-row transition after processing
completion is treated as an idempotent no-op rather than throwing
DuplicateRecordProblem. Preserve and rethrow the original QuotaExceededProblem,
while distinguishing lease/token fencing or KV-mismatch failures with a separate
diagnostic Problem or code.

In `@packages/metering-core/src/libs/MeteringService.ts`:
- Around line 233-247: Update the publishDelivery/recordUsage error path so that
once completeMeteringProcessing succeeds, a later quotaManager.validateOrThrow
failure is recognized as completed rather than still claimed. Propagate
completion status from publishDelivery or detect the COMPLETED state before
cleanup, and prevent releaseMeteringEvents from attempting cleanup or reverting
the delivery to PUBLISHING in this case.

In `@packages/metering-core/src/tests/IdempotencyManager.spec.ts`:
- Around line 152-167: Update the script handler in InMemoryRedisClient so
unsupported Lua scripts throw an error instead of falling through to the final
successful [1] response. Preserve the existing returns for recognized DEL and
other supported script branches, but remove or replace the unconditional
fallback success path.

In `@packages/metering-core/src/tests/MeteringService.spec.ts`:
- Around line 674-683: Strengthen the retry deduplication regression tests in
packages/metering-core/src/tests/MeteringService.spec.ts: for lines 674-683,
increment the checkAndRecordWithinQuota call counter on every invocation and
separately assert total calls versus first-time records per
usageRecord.idempotencyKey; for lines 585-613, key persistedOperations by
usage.idempotencyKey, ignore repeated keys while tracking total record calls,
and assert both values; for lines 625-660, key records by
usageRecord.idempotencyKey and verify the second call reuses the first quota
result.

In `@scripts/quick-start-lambda-smoke.mts`:
- Around line 199-213: Update assertServerOutput to briefly poll for
expectedText after the HTTP response, allowing pending child-process stdout data
events to arrive before failing. Preserve the existing label-specific error and
success logging, and time out with the current assertion error if the text never
appears.

---

Outside diff comments:
In `@packages/metering-core/src/libs/RedisUsageStorage.ts`:
- Around line 97-119: Update the legacy dedupe fallback in the Lua logic used by
RedisUsageStorage so a stored value such as "1" does not unconditionally return
exceeded=false. When recordedResult is present but the quota-formatted parse
fails, recompute the exceeded flag from currentUsage using the same quota limit
and return that result with currentUsage, preserving the existing behavior for
valid quota-formatted dedupe values.

In `@packages/metering-core/src/tests/RedisMetering.integration.spec.ts`:
- Around line 232-235: Update both failure assertions in
RedisMetering.integration.spec.ts at lines 232-235 and 261-264 to first apply
rejects.toThrow() to the same Promise, while preserving the existing
rejects.toMatchObject structure checks for the error code and operation.
- Around line 221-225: Update the restrictedClient setup in the Redis metering
integration test to use the same key prefix as connection.client, specifically
connection.keyPrefix, instead of an empty prefix. Keep the retry and
verification paths unchanged so both clients target the identical physical Redis
key.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c4bfcbe0-107a-45c8-adb5-058a3525e090

📥 Commits

Reviewing files that changed from the base of the PR and between 8ee1f1b and e82af59.

📒 Files selected for processing (41)
  • .changeset/calm-geckos-recover.md
  • examples/quick-start-lambda/src/integrations/inMemoryMetering.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/inMemoryAdapters.ts
  • packages/create-croco-app/templates/saas/apps/api-server/src/tests/SaasDemo.spec.ts
  • packages/docs/src/content/docs/api/credits-core/src/classes/CreditLedgerStore.md
  • packages/docs/src/content/docs/api/events-core/src/classes/DomainEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/IdempotencyManager.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/QuotaExceededEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/RedisUsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/classes/UsageRecordedEvent.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/RedisClient.md
  • packages/docs/src/content/docs/api/metering-core/src/interfaces/UsageStorage.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/MeteringProcessingClaim.md
  • packages/docs/src/content/docs/api/metering-core/src/type-aliases/PendingMeteringDelivery.md
  • packages/docs/src/content/docs/api/metering-upstash/src/classes/UpstashRedisClient.md
  • packages/docs/src/content/docs/api/problems-core/src/classes/Problem.md
  • packages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.md
  • packages/events-core/src/libs/DomainEvent.ts
  • packages/events-core/src/tests/DomainEvents.spec.ts
  • packages/metering-core/README.md
  • packages/metering-core/src/index.ts
  • packages/metering-core/src/libs/IdempotencyManager.ts
  • packages/metering-core/src/libs/MeteringService.ts
  • packages/metering-core/src/libs/QuotaManager.ts
  • packages/metering-core/src/libs/RedisClient.ts
  • packages/metering-core/src/libs/RedisUsageStorage.ts
  • packages/metering-core/src/libs/UsageStorage.ts
  • packages/metering-core/src/libs/events/QuotaExceededEvent.ts
  • packages/metering-core/src/libs/events/UsageRecordedEvent.ts
  • packages/metering-core/src/libs/events/eventIdentity.ts
  • packages/metering-core/src/tests/IdempotencyManager.spec.ts
  • packages/metering-core/src/tests/MeteringService.spec.ts
  • packages/metering-core/src/tests/QuotaManager.spec.ts
  • packages/metering-core/src/tests/RedisMetering.integration.spec.ts
  • packages/metering-core/src/tests/RedisUsageStorage.spec.ts
  • packages/metering-core/src/tests/UsageAggregator.spec.ts
  • packages/metering-core/src/tests/UsageStorage.spec.ts
  • packages/metering-core/src/tests/events/Events.spec.ts
  • packages/metering-upstash/src/libs/UpstashRedisClient.ts
  • public-api-surface.snapshot.json
  • scripts/quick-start-lambda-smoke.mts
💤 Files with no reviewable changes (1)
  • packages/metering-core/src/libs/QuotaManager.ts

Comment thread packages/metering-core/src/libs/events/eventIdentity.ts
Comment thread packages/metering-core/src/libs/IdempotencyManager.ts
Comment thread packages/metering-core/src/libs/IdempotencyManager.ts Outdated
Comment thread packages/metering-core/src/libs/MeteringService.ts
Comment thread packages/metering-core/src/tests/IdempotencyManager.spec.ts
Comment thread packages/metering-core/src/tests/MeteringService.spec.ts Outdated
Comment thread scripts/quick-start-lambda-smoke.mts Outdated
@kang-heewon
kang-heewon force-pushed the fix/1574-recover-metering-events branch from e82af59 to 2174216 Compare July 30, 2026 17:44
@kang-heewon
kang-heewon force-pushed the fix/1574-recover-metering-events branch from 2174216 to 607041e Compare July 30, 2026 18:32
@kang-heewon
kang-heewon merged commit 88c6ce1 into trunk Jul 30, 2026
19 of 20 checks passed
@kang-heewon
kang-heewon deleted the fix/1574-recover-metering-events branch July 30, 2026 19:45
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.

[metering-core][regression] Completing idempotency before event publication makes delivery failures unrecoverable

1 participant