Skip to content

feat(metadata): protect hardcover daily quotas (#2593) - #2607

Closed
magrhino wants to merge 0 commit into
vavallee:mainfrom
magrhino:codex/implement-hardcover-quota-protection
Closed

magrhino wants to merge 0 commit into
vavallee:mainfrom
magrhino:codex/implement-hardcover-quota-protection

Conversation

@magrhino

Copy link
Copy Markdown
Contributor

Summary

Hardcover daily exhaustion currently allows integrations to keep attempting work against an unavailable allowance. This change shares persistent quota accounting across Hardcover consumers, detects the actual plan allowance from documented API signals, and defers work until it is eligible again without losing provider identity or pending edition hydration.

Closes #2593. Refs #2592.

  • Count outgoing attempts, retries, pages, and account discovery before transmission; share accounting and adaptive pacing by verified account, with isolated key fallback.
  • Honor documented daily allowance/remaining/reset signals and persisted exhaustion holds. Use a configurable 5,000-request fallback when detection is unavailable (50,000 for Supporter), with conservative rolling local accounting and no assumed midnight reset. Reference: Hardcover Supporter.
  • Reserve 10% for interactive work, expose admin quota status, and retain safe deferred work through existing scheduled/manual entry points.
  • Add migration 086 for book-owned deferred edition work, including cascading cleanup and legacy marker migration; document configuration and upgrade behavior.

Scope

  • This PR has one reviewable objective: daily quota protection.
  • Follow-up work is called out: Caching and request deduplication #2594 owns caching and request deduplication. It is excluded here. Integration must retain quota state in token-bound clients, avoid charging cache hits/shared waiters, and test mixed interactive/background request sharing.

Accounting coordinates consumers within one Bindery process/database. Independent installations cannot share local counters. Resume uses existing scheduled/manual entry points; no immediate-resume job is introduced. Supported signals were verified against official API documentation; no authenticated live quota measurement was made.

Suggested review order

This is a 30-file change (+1,862/-45), including focused regression tests and documentation. Suggested order:

  1. internal/metadata/hardcover/quota.go and its tests: accounting, discovery, admission, reset handling, and pacing.
  2. Client wiring and API/list/scheduler deferral propagation.
  3. Migration 086, settings persistence, and hydration recovery tests: provider identity, pinned format, deletion, and late completion.
  4. docs/Hardcover-Quota.md and deployment/wiki updates.

Checklist

  • Commit signed off with git commit -s.
  • Tests added or updated.
  • README, configuration/upgrade documentation, and Hardcover wiki updated.
  • Changelog fragment added under changelog.d/.
  • template.env — N/A: configuration is a runtime SQLite-backed setting, not an environment variable.
  • CHANGELOG.md — N/A: repository policy reserves versioned changelog edits for releases; fragment supplied instead.

Test plan

All listed completed commands passed (exit 0). Machine-specific cache paths are omitted below; focused tests, race tests, vet, and lint used Go 1.26.6, and lint used golangci-lint v2.11.4.

  • go test ./internal/metadata/hardcover ./internal/metadata ./internal/db ./internal/api ./internal/bookhydrate ./internal/hardcoverlistsyncer -run 'Test.*Quota|TestDeferredHardcover|TestMigration086|TestEditionPrefetchDeferred|TestThrottle|TestHydrateHardcover' -count=1 — six packages passed.
  • go test -race ./internal/metadata/hardcover ./internal/db ./internal/api ./internal/bookhydrate -run 'Test.*Quota|TestDeferredHardcover|TestMigration086|TestEditionPrefetchDeferred|TestThrottle|TestHydrateHardcover' -count=1 — four packages passed.
  • make test — full backend suite passed.
  • make smoke — rebuilt frontend and binary; smoke suite passed. Vite reported a non-fatal large-chunk warning.
  • go vet ./... — no diagnostics.
  • golangci-lint run --timeout=5m — zero issues.
  • git diff --check and staged diff check — passed; changed Go files are gofmt-clean.
  • tests/run-all.sh — not run; validation used the repository's make targets and focused commands above.
  • Docker/Compose validation — N/A: no container configuration changes.

The final concurrency/persistence fixes received focused independent review with no findings. A fresh full-feature review was not rerun after those fixes. Only deployment/wiki documentation links were added after the completed runtime validation.

@github-actions github-actions Bot added the bindery-notified Discord notification already sent for this PR label Sep 14, 2026
Comment thread internal/metadata/hardcover/quota.go Outdated
func (q *Quota) observe(s *quotaState, h http.Header, code int) {
policy := dailyParameters(strings.Join(h.Values("RateLimit-Policy"), ","))
if policy["q"] > 0 && policy["q"] <= 1_000_000_000 && policy["w"] == 86400 {
limit := int(policy["q"])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe this is a false positive.

@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

@vavallee vavallee left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this, @magrhino, and for doing the homework on the premise. I checked it against the docs source (hardcover-docs, api/Getting-Started.mdx): the 5,000 and 50,000 daily limits, the RateLimit and RateLimit-Policy headers, and the 429 once the daily limit is spent are all there, and quota.go parses them correctly. The CodeQL alert on quota.go:451 is a false positive too, since the value is bounded.

I'd like to get protection for this in, but in a much smaller form. Some things turned up when I merged it onto current main and read it closely.

Blocking

  1. It no longer builds against main. #2641 changed GetBookByISBNWithOutcome to return an outcome, so aggregator.go:921 has return nil, err where three values are needed.
  2. Daily exhaustion doesn't stop imports. QuotaDeferredError matches ErrRateLimited (quota.go:99), and since v1.36.2 the import outage breaker deliberately ignores ErrRateLimited, so short throttling doesn't fail a whole import. A spent daily quota lasts hours, so an import keeps going row by row. Exhaustion needs its own error, and imports should stop on it with the reset time.
  3. Every Hardcover call in the process runs one at a time. The gate is held through the network request (quota.go:246 to 391), across all users' tokens, and each call also costs about six settings reads and three writes.
  4. A short per minute 429 turns into a long deferral. A Retry-After between 31 and 60 seconds sets Hold, and hydration then stores a deferral marker instead of retrying.
  5. Deferred edition hydration only resumes from manual refreshes. retryDeferredEditions runs from fetchAuthorBooks on an existing match, and all the callers that reach it are manual. Books added through the book, series or recommendation flows can stay half hydrated.
  6. The interactive reserve is uneven. Only list sync, refreshMetadata and fetchAuthorBooks are tagged background. ABS, CSV and Goodreads imports count as interactive and can use up the reserve, while a user adding an author gets a background catalogue sync that is skipped with only a log line.

Smaller things

  • There's no UI, so the status is only visible at GET /setting/hardcover/quota.
  • Migration 086 collides with #2633's 086.
  • The legacy marker migration moves auth.hardcover_deferred_editions.* keys that don't exist on main. They look like they're from an earlier draft of this PR, so it can go.

What I'd suggest

Nobody has reported hitting the daily limit yet. The closest is #2075, about bulk CSV load. So I'd rather start with the part that covers the real risk, a bulk job hammering an exhausted key:

  • parse the documented RateLimit headers;
  • persist a hold until the daily reset when the remaining count reaches zero or the daily 429 arrives;
  • give that its own error that stops imports and background jobs, leaving the existing short throttle handling alone;
  • add one line in Settings saying Hardcover is paused until a given time.

That should be a few hundred lines. The shared accounting, the interactive reserve and deferred hydration can follow once someone actually runs into the limit, and they'll be easier to review on top of that base. On ordering, this should land before #2615, since the caching PR builds on the quota carrying client.

If you'd prefer to cut this PR down in place rather than open a new one, that's fine by me. Thanks again, the header work is the hard part and it's right.

@magrhino

magrhino commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

@vavallee Thanks for the direction. I’ve opened #2699 with the smaller daily-reset hold and removed the accounting, reserve, and deferred-hydration pieces. For context, I encountered the original issue after a full ABS library preview followed by a real sync, but didn’t capture an error, so I can't state the exact cause just that my hardcover api was capped. The cache fixes may also address that original behavior.

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

Labels

bindery-notified Discord notification already sent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daily quota protection

3 participants