Skip to content

perf(service): cut request overhead and serve now playing as a CDN-cached shared read - #3182

Merged
Chia1104 merged 2 commits into
developfrom
perf/service-request-overhead
Oct 1, 2026
Merged

Chia1104 merged 2 commits into
developfrom
perf/service-request-overhead

Conversation

@Chia1104

@Chia1104 Chia1104 commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Why

Production traces (Tempo 24h, Railway 7d) show service handlers are fast — feeds.list p50 ~47ms, feeds.details-by-slug p50 ~120ms — and the time www sees goes to what surrounds them:

  • No Access-Control-Max-Age, so browsers re-preflight every oRPC POST (~288 OPTIONS/day, ~200ms each from Taipei).
  • pg's 10s idle timeout empties the pool between most requests: ~506 new connections per ~820 RPC calls/day, ~25ms each.
  • spotify.playing, the most-called procedure (289/day), hit api.spotify.com on every request and returned Spotify's raw payload, while every visitor polls the same data.

Railway's p95/p99 of 30000ms are long-lived streams (/api/v1/mcp SSE, feeds/draft:watch, agent chat), not slow requests.

Changes

CORS and pool

  • cors() sets maxAge: 7200 (the Chromium cap).
  • The pool keeps idle clients for 5 minutes, with allowExitOnIdle so migrate.ts, which never ends the pool, still exits.

spotify.playing as a shared read

  • New @chia/services/shared/shared-reads: procedures whose answer is the same for every caller, mapped to the CDN-Cache-Control a success carries. The module imports nothing, so both the handler and the browser link load it.

  • service accepts GET only for those (allowMethods). A successful GET carries:

    • CDN-Cache-Control: max-age=10, stale-while-revalidate=50 for Cloudflare (s-maxage would turn off stale-while-revalidate there)
    • Cache-Control: no-cache for browsers
    • Access-Control-Allow-Origin: * without credentials, because a CDN stores one copy for every origin and ignores Vary: Origin

    These are set through c.header, because Hono copies the CORS headers set before next onto the returned response. Failures carry no cache headers.

  • The www link sends shared reads as credential-less GET …/spotify/playing?data= with no custom headers, so there is no preflight and every visitor shares one cache key.

  • The contract returns { track, isPlaying, progressMs, observedAt } instead of Spotify's raw payload. The response is null for ads and podcasts.

  • service caches the DTO in Redis for 10s and treats a track that has ended as stale.

  • www advances progress from observedAt and refetches when the track should end: at least 5s, so a stale edge copy cannot make it spin, and at most 60s, so skips and pauses still show. This replaces the progress context and the end-of-track refetch loop.

Cloudflare (already applied)

  • Zone cache rule "service shared reads edge cache": GETs on service.chia1104.dev under /api/v1/rpc/ are cache-eligible. The edge follows the response's cache headers and bypasses the cache when there are none. Checked against current production: the GET still gets 404 + BYPASS, and POST and health stay DYNAMIC.

Deploy notes

  • The spotify.playing response shape changed and there is no compatibility layer. Until both sides are deployed, and in tabs still running the old www bundle, the footer now-playing widget errors until reload.
  • After deploy, repeat GET https://service.chia1104.dev/api/v1/rpc/spotify/playing?data=. Expect cf-cache-status MISS, then HIT within 10s and UPDATING from 10 to 60s. BYPASS would mean Cloudflare is not reading CDN-Cache-Control.
  • Re-measure with the same Tempo queries: OPTIONS count, pg.connect per RPC call, and api.spotify.com calls under spotify.playing.

Tests

  • apps/service: 129 passed, 1 skipped. New cases cover the DTO mapping, null for ads, the shared cache window, refetch after the track ends, GET headers (CDN + browser + CORS), no cache headers on POST or a failed GET, and 404 for GET on a procedure that is not a shared read.
  • @chia/services 133, @chia/db, @chia/service-kit and www tests pass.
  • Type check and lint with --force pass for db, service-kit, services, service, workflow-service and www.
  • Not exercised in a browser.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Spotify now-playing information is cached briefly and includes normalized track details, playback progress, and observation time.
    • Shared read requests can be served through credential-free GETs and CDN caching.
    • Playback progress updates while a track is playing and remains fixed while paused.
  • Bug Fixes

    • Ads and unsupported playback items no longer appear as tracks.
    • Playlist requests return 404 instead of calling the playlist service when unavailable.
    • Now-playing errors avoid returning cache headers.

Chia1104 and others added 2 commits October 1, 2026 13:58
Browsers re-sent the preflight for every oRPC POST because the CORS
response carried no Access-Control-Max-Age; cache it for 7200s, the
Chromium cap.

pg's 10s idle timeout closed the pool between most requests at this
traffic (~506 new connections per ~820 RPC calls a day, ~25ms each).
Keep idle clients for 5 minutes, with allowExitOnIdle so one-shot
scripts such as migrate.ts still exit.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
spotify.playing is the same for every visitor, so it no longer rides
the per-visitor POST path:

- shared/shared-reads.ts lists procedures that answer every caller
  alike. service accepts GET only for those and marks a success with
  CDN-Cache-Control, browser Cache-Control: no-cache and
  Access-Control-Allow-Origin: *; the www link sends them as
  credential-less GETs, which need no preflight.
- The output is a small DTO (track, isPlaying, progressMs, observedAt)
  instead of Spotify's raw payload. The browser advances progress from
  observedAt and refetches when the track should end.
- service caches the DTO in Redis for 10s and treats a track that has
  ended as stale.

A Cloudflare cache rule now makes GETs under /api/v1/rpc/ cacheable
when the response carries cache headers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
chia1104 Ready Ready Preview Oct 1, 2026 5:59am UTC

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (3)
apps/AGENTS.md — auto-discovered
packages/AGENTS.md — auto-discovered
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5eed90cd-8b81-49dd-b81b-e684d6ef4f72

📥 Commits

Reviewing files that changed from the base of the PR and between bef762c and 00fd973.

📒 Files selected for processing (14)
  • apps/AGENTS.md
  • apps/service/__tests__/spotify.controller.test.ts
  • apps/service/__tests__/spotify.service.test.ts
  • apps/service/src/routes/rpc.route.ts
  • apps/www/src/components/commons/current-playing.tsx
  • apps/www/src/libs/orpc/client.ts
  • packages/db/src/client.ts
  • packages/service-kit/src/bootstrap.ts
  • packages/services/AGENTS.md
  • packages/services/package.json
  • packages/services/shared/shared-reads.ts
  • packages/services/spotify/playback.service.ts
  • packages/services/spotify/spotify.contract.ts
  • packages/services/spotify/spotify.route.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds a shared, cached Spotify playback read with a normalized response. It serves that read through credential-less GET requests with CDN headers and updates the playback display. It also changes PostgreSQL idle connection settings and CORS preflight caching.

Changes

Shared Spotify playback read

Layer / File(s) Summary
Shared-read and playback contracts
packages/services/shared/shared-reads.ts, packages/services/package.json, packages/services/spotify/spotify.contract.ts, packages/services/AGENTS.md
Defines spotify.playing as a shared read with a CDN cache policy, exports the shared-read map, and defines a nullable normalized playback response schema.
Playback mapping and cache
packages/services/spotify/playback.service.ts, packages/services/spotify/spotify.route.ts, apps/service/__tests__/spotify.service.test.ts
Maps Spotify playback data to the normalized response. The service uses Keyv to reuse cached responses until expiry or until a playing track has ended. Tests cover mapping, caching, ads, idle playback, and authorization retry.
Credential-less GET and CDN response
apps/service/src/routes/rpc.route.ts, apps/www/src/libs/orpc/client.ts, packages/service-kit/src/bootstrap.ts, apps/service/__tests__/spotify.controller.test.ts, apps/AGENTS.md
Allows GET for shared reads, adds cache and CORS headers to successful shared-read GET responses, and omits credentials for those browser requests. CORS preflight caching is set to 7200 seconds. Controller tests cover the response headers and GET behavior.
Normalized playback display
apps/www/src/components/commons/current-playing.tsx
Uses normalized track fields and estimates playback progress from the observation time. The component updates progress while playing and schedules refetches based on the remaining track time.

PostgreSQL connection settings

Layer / File(s) Summary
PostgreSQL client configuration
packages/db/src/client.ts
Passes the connection URL, five-minute idle timeout, and allowExitOnIdle: true to the PostgreSQL client.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant RpcHandler
  participant SpotifyRoute
  participant PlaybackService
  participant Keyv
  participant SpotifyAPI
  Browser->>RpcHandler: GET spotify.playing
  RpcHandler->>SpotifyRoute: Invoke playback route
  SpotifyRoute->>PlaybackService: Pass database and KV contexts
  PlaybackService->>Keyv: Read cached playback
  alt Cache miss or cached track ended
    PlaybackService->>SpotifyAPI: Fetch current playback
    SpotifyAPI-->>PlaybackService: Return Spotify playback data
    PlaybackService->>Keyv: Store normalized response
  else Cached response is reusable
    Keyv-->>PlaybackService: Return cached response
  end
  PlaybackService-->>SpotifyRoute: Return normalized playback
  SpotifyRoute-->>RpcHandler: Return procedure result
  RpcHandler-->>Browser: Return playback response
Loading

Merge Risk: ⚪ Minimal · up to 00fd9

The shared playback changes are mergeable after normal checks. Redis cache-write failures do not discard successful playback reads under the production configuration.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 00fd9

The shared read is restricted to playback metadata and does not depend on the visitor’s identity. However, switching or disconnecting the published Spotify account does not invalidate its cached response, so previously published metadata can remain available temporarily. Production CDN behavior has not been confirmed.

Retained concerns

  • Low · security · inferred: The new shared playback cache is not coupled to the published account’s lifecycle. Cache hits return metadata without checking the active credential, while activation and disconnect change database ownership without invalidating the cache. Consequently, a completed disconnect or account switch can still be followed by public delivery of the previous account’s metadata. The service TTL limits ordinary persistence to its remaining cache window, but the edge policy requests additional stale delivery and its effective behavior is unverified. This concerns continued publication of previously public metadata, not disclosure of credentials or arbitrary users’ Spotify data.
Security review details

Security Blast Radius

  • inferred — The supported exposure is shared playback metadata for the globally published account, available to anonymous callers and cross-origin readers. The inspected producer does not use caller identity to retrieve private visitor accounts. Whether separate deployments share the same cache infrastructure is not established.

Security Findings and Attack Paths

  • inferred — After an administrator switches or disconnects the active account, an anonymous reader can still obtain a previously cached observation because a cache hit skips credential lookup. This is an introduced stale-publication path; it does not establish access to unpublished observations or credential material.

Trust Boundaries and Controls

  • observed — Origin-side caller resolution and rate limiting remain before RPC dispatch. Only registry-listed procedures accept GET; only successful shared GET responses receive cache directives. The browser omits credentials for those requests, and the service removes cross-origin credential allowance.

Resilience and Maintainability Implications

  • inferred — The cache miss sequence has no generation check between fetch and write. An in-flight fetch begun before an account transition can therefore populate the cache afterward. Simple deletion alone would not fully enforce withdrawal against concurrent producers.

Hardening Proposals

  • proposed — Define the intended withdrawal delay explicitly. If account transitions must stop publication promptly, couple cached observations to a publication generation, reject stale-generation writes, and coordinate origin invalidation with an appropriate edge purge or cache-key change.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 11 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: reducing service request overhead and serving Spotify now playing as a CDN-cached shared read.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 11 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@Chia1104
Chia1104 merged commit 8826fa2 into develop Oct 1, 2026
15 checks passed
@Chia1104
Chia1104 deleted the perf/service-request-overhead branch October 1, 2026 06:16

This branch was successfully deployed

1 active deployment
Preview — 00fd973f Deployed Oct 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant