Skip to content

Fix logs#3761

Merged
steven-tey merged 7 commits intomainfrom
fix-logs
Apr 16, 2026
Merged

Fix logs#3761
steven-tey merged 7 commits intomainfrom
fix-logs

Conversation

@devkiran
Copy link
Copy Markdown
Collaborator

@devkiran devkiran commented Apr 16, 2026

Summary by CodeRabbit

  • New Features

    • Added a "Logs" page under Developer settings.
  • Improvements

    • Request logging now runs in the background so API responses return without waiting for log persistence (reduced perceived latency).
  • Chores

    • Reserved "undefined" and "null" as unavailable workspace/slug names.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 16, 2026

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

Project Deployment Actions Updated (UTC)
dub Ready Ready Preview Apr 16, 2026 7:53pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d21c0031-7549-4559-9070-7263f516e75d

📥 Commits

Reviewing files that changed from the base of the PR and between 20be702 and 50ec365.

📒 Files selected for processing (7)
  • apps/web/app/(ee)/api/track/open/route.ts
  • apps/web/lib/api-logs/capture-request-log.ts
  • apps/web/lib/api-logs/capture-webhook-log.ts
  • apps/web/lib/auth/publishable-key.ts
  • apps/web/lib/auth/workspace.ts
  • apps/web/ui/layout/sidebar/app-sidebar-nav.tsx
  • packages/utils/src/constants/reserved-slugs.ts

📝 Walkthrough

Walkthrough

Request logging calls were moved from synchronous execution into Vercel's background task mechanism (waitUntil) across several API routes and auth helpers; captureRequestLog was made async. A "Logs" item was added to the workspace settings sidebar and "undefined"/"null" were appended to reserved slugs.

Changes

Cohort / File(s) Summary
Request logging / routes
apps/web/app/(ee)/api/track/open/route.ts, apps/web/lib/auth/publishable-key.ts, apps/web/lib/auth/workspace.ts
Switched request-log calls to waitUntil(captureRequestLog(...)), deferring log capture to Vercel background tasks; workspace.ts clears the invalid workspace before throwing in the missing-workspace branch.
Logging implementation
apps/web/lib/api-logs/capture-request-log.ts, apps/web/lib/api-logs/capture-webhook-log.ts
Made captureRequestLog an async function and converted inline deferred code to awaited recordApiLog(...); one formatting blank-line change in capture-webhook-log.ts.
UI — Sidebar
apps/web/ui/layout/sidebar/app-sidebar-nav.tsx
Added StackY3 import and inserted a "Logs" navigation entry under Settings → Developer linking to /${slug}/settings/logs.
Constants
packages/utils/src/constants/reserved-slugs.ts
Appended "undefined" and "null" to exported RESERVED_SLUGS array.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RouteHandler as Route Handler
    participant VercelWA as waitUntil (Vercel)
    participant Capture as captureRequestLog
    participant Store as recordApiLog / DB

    Client->>RouteHandler: HTTP POST request
    RouteHandler->>RouteHandler: process request, build response
    RouteHandler->>VercelWA: waitUntil(captureRequestLog(req,...))
    VercelWA->>Capture: invoke captureRequestLog(req, response, meta)
    Capture->>Store: recordApiLog(request/response data)
    RouteHandler-->>Client: return response (without waiting for Capture)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Suggested reviewers

  • pepeladeira

Poem

🐰 I hopped through code at break of dawn,

Deferred the logs, then swiftly yawned.
"Logs" now nest in the settings tree,
Reserved words shooed — null and undefined, whee! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
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.
Title check ❓ Inconclusive The title 'Fix logs' is vague and generic, using non-descriptive terms that don't convey meaningful information about the changeset beyond that it involves logging. Consider a more specific title that describes the primary refactor, such as 'Refactor request logging to use async/await with deferred execution' or 'Move waitUntil to request logging call sites'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-logs

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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/lib/api-logs/record-api-log.ts`:
- Line 53: Read paths are passing the raw workspaceId to Tinybird while
record-api-log.ts stores workspace_id via prefixWorkspaceId(workspaceId); update
get-api-logs.ts and get-api-logs-count.ts to apply the same normalization by
transforming workspaceId before querying (e.g., call
prefixWorkspaceId(workspaceId) or map it in the Tinybird query/schema similar to
get-audit-logs.ts). Locate the Tinybird parameter construction in
get-api-logs.ts (around the parameter named workspaceId) and in
get-api-logs-count.ts, replace the unprefixed value with the output of
prefixWorkspaceId(workspaceId) or add a schema-level alias so Tinybird queries
use workspace_id that matches record-api-log.ts.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: c75296a2-1069-41be-a5bc-0575f0a8afe3

📥 Commits

Reviewing files that changed from the base of the PR and between 073f59f and b43de81.

📒 Files selected for processing (1)
  • apps/web/lib/api-logs/record-api-log.ts

Comment thread apps/web/lib/api-logs/record-api-log.ts Outdated
id: createId({ prefix: "req_" }),
timestamp: new Date().toISOString(),
workspace_id: workspaceId,
workspace_id: prefixWorkspaceId(workspaceId),
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Tinybird pipe references =="
rg -n -C3 'get_api_logs' --glob '!**/node_modules/**'

echo
echo "== API log read-path callsites involving workspaceId =="
rg -n -C3 'getApiLogs|workspaceId|workspace_id' --type=ts --type=tsx --glob '!**/node_modules/**'

echo
echo "== Normalization callsites =="
rg -n -C3 'prefixWorkspaceId\\(' --type=ts --type=tsx --glob '!**/node_modules/**'

Repository: dubinc/dub

Length of output: 1105


🏁 Script executed:

cat -n apps/web/lib/api-logs/get-api-logs.ts

Repository: dubinc/dub

Length of output: 1499


🏁 Script executed:

cat -n apps/web/lib/api-logs/get-api-logs-count.ts

Repository: dubinc/dub

Length of output: 2091


🏁 Script executed:

cat -n apps/web/lib/api-logs/record-api-log.ts | head -60

Repository: dubinc/dub

Length of output: 1991


🏁 Script executed:

# Search for callsites of getApiLogs and getApiLogsCount functions
rg -n 'getApiLogs|getApiLogsCount' apps/web --type ts

Repository: dubinc/dub

Length of output: 2160


🏁 Script executed:

cat -n apps/web/app/api/logs/route.ts

Repository: dubinc/dub

Length of output: 1335


🏁 Script executed:

cat -n apps/web/app/api/logs/count/route.ts

Repository: dubinc/dub

Length of output: 1168


🏁 Script executed:

# Search for prefixWorkspaceId usage in the codebase
rg -n 'prefixWorkspaceId' apps/web --type ts

Repository: dubinc/dub

Length of output: 4753


Fix workspace ID normalization in API logs read paths.

The write path (record-api-log.ts:53) now prefixes workspace_id, but the read paths pass unprefixed workspaceId to Tinybird:

  • get-api-logs.ts:38 passes workspaceId directly
  • get-api-logs-count.ts:34 passes workspaceId directly

This mismatch means newly ingested rows with prefixed IDs won't be returned by queries. Apply the same normalization pattern used in get-audit-logs.ts by transforming workspaceId in the schema or before querying.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/lib/api-logs/record-api-log.ts` at line 53, Read paths are passing
the raw workspaceId to Tinybird while record-api-log.ts stores workspace_id via
prefixWorkspaceId(workspaceId); update get-api-logs.ts and get-api-logs-count.ts
to apply the same normalization by transforming workspaceId before querying
(e.g., call prefixWorkspaceId(workspaceId) or map it in the Tinybird
query/schema similar to get-audit-logs.ts). Locate the Tinybird parameter
construction in get-api-logs.ts (around the parameter named workspaceId) and in
get-api-logs-count.ts, replace the unprefixed value with the output of
prefixWorkspaceId(workspaceId) or add a schema-level alias so Tinybird queries
use workspace_id that matches record-api-log.ts.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@coderabbitai great catch, thank you!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@steven-tey, happy to help! To summarize the fix needed — both read paths need to apply the same prefixWorkspaceId() normalization before querying Tinybird:

get-api-logs.ts (around line 38):

workspace_id: prefixWorkspaceId(workspaceId),

get-api-logs-count.ts (around line 34):

workspace_id: prefixWorkspaceId(workspaceId),

Make sure to import prefixWorkspaceId from ../api/workspaces/workspace-id in both files, mirroring the pattern already used in get-audit-logs.ts. This will ensure newly ingested rows (with prefixed IDs) are correctly returned by queries.


🧠 Learnings used
Learnt from: steven-tey
Repo: dubinc/dub PR: 3679
File: apps/web/app/(ee)/api/discount-codes/route.ts:69-90
Timestamp: 2026-04-01T17:57:15.397Z
Learning: In the dubinc/dub codebase, a workspace can only ever have one installed Stripe integration at a time. Therefore, `prisma.installedIntegration.findFirst({ where: { projectId, integrationId: STRIPE_INTEGRATION_ID } })` is always deterministic and there is no risk of returning the wrong row. Do not flag this pattern as nondeterministic in future reviews.

Learnt from: devkiran
Repo: dubinc/dub PR: 3346
File: apps/web/lib/api/bounties/approve-bounty-submission.ts:165-165
Timestamp: 2026-01-13T12:06:42.476Z
Learning: Use 'noreply' as a sentinel for the replyTo email field: if replyTo === 'noreply', omit the replyTo property by spreading an empty object. Call sites can use replyTo: someEmail || 'noreply' to conditionally set replyTo without null/undefined. This pattern is broadly applicable across the TypeScript codebase; ensure this behavior is documented and that readability is preserved, using explicit conditional logic if the intent may be unclear.

Learnt from: devkiran
Repo: dubinc/dub PR: 3358
File: apps/web/app/(ee)/api/cron/partners/deactivate/route.ts:7-7
Timestamp: 2026-02-03T14:14:47.579Z
Learning: In all TypeScript files under apps/web, import zod using a namespace import: import * as z from 'zod/v4' and avoid using named import: import { z } from 'zod'. This ensures consistent API access and version specificity. During reviews, look for and replace any occurrences of import { z } from 'zod' with import * as z from 'zod/v4', and verify all zod usage continues to compile.

Learnt from: marcusljf
Repo: dubinc/dub PR: 3671
File: apps/web/ui/partners/program-card.tsx:93-93
Timestamp: 2026-03-31T20:45:16.716Z
Learning: In this codebase, Tailwind’s normal font weight (400) must use `font-normal`. Do not use `font-regular` (it is not a valid Tailwind utility here). When reviewing Tailwind class strings in `.ts`/`.tsx` files, flag any occurrence of `font-regular` and replace it with `font-normal` when the intent is weight 400.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@steven-tey
Copy link
Copy Markdown
Collaborator

@coderabbitai full review, any breaking changes?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

🧩 Analysis chain

I'll do a thorough analysis of the code before providing the full review.


🏁 Script executed:

#!/bin/bash
echo "=== All callers of captureRequestLog ==="
rg -n "captureRequestLog" --type=ts -A2 -B2

echo ""
echo "=== All callers of captureWebhookLog ==="
rg -n "captureWebhookLog" --type=ts -A2 -B2

echo ""
echo "=== Full content of capture-request-log.ts ==="
fd "capture-request-log.ts" --exec cat {}

echo ""
echo "=== Full content of record-api-log.ts ==="
fd "record-api-log.ts" --exec cat {}

echo ""
echo "=== Full content of workspace.ts (auth) ==="
fd "workspace.ts" --search-path apps/web/lib/auth --exec cat {}

echo ""
echo "=== Full content of publishable-key.ts ==="
fd "publishable-key.ts" --search-path apps/web/lib/auth --exec cat {}

echo ""
echo "=== Full content of track/open route.ts ==="
fd "route.ts" --search-path "apps/web/app/(ee)/api/track/open" --exec cat {}

echo ""
echo "=== prefixWorkspaceId usages ==="
rg -n "prefixWorkspaceId" --type=ts -A2 -B2

echo ""
echo "=== app-sidebar-nav.tsx Logs section ==="
fd "app-sidebar-nav.tsx" --exec grep -n -A5 -B5 "Logs\|StackY3" {}

Length of output: 54543


@steven-tey Here's a thorough review of PR #3761. No breaking changes to external API consumers. Here's the full breakdown:


Summary of Changes

The core change is a refactoring of how captureRequestLog is executed: waitUntil was moved from inside captureRequestLog to all call sites, and captureRequestLog was promoted to async. This is a clean architectural improvement that makes the async/background-execution boundary explicit at each call site.


File-by-File Review

apps/web/lib/api-logs/capture-request-log.ts

Clean. The function is now async and directly awaits recordApiLog. The removal of the internal waitUntil is intentional — callers now own that responsibility. The early-return guard (!isMutation || routePattern === "unknown") correctly short-circuits before any I/O.

apps/web/lib/auth/workspace.ts

Correct. Both the success path and the error path now use waitUntil(captureRequestLog(...)). The workspace = undefined assignment before throwing DubApiError (when workspace lookup fails) is a good bug fix — it prevents the catch block from logging errors against an invalid/partial workspace reference.

apps/web/lib/auth/publishable-key.ts

Correct. Both paths updated consistently.

apps/web/app/(ee)/api/track/open/route.ts

Correct. Both the success and error paths updated. The error path is guarded by if (workspaceId) before calling waitUntil, which is appropriate since workspaceId may not be populated for early errors.

apps/web/lib/api-logs/record-api-log.ts

⚠️ Note on AI summary discrepancy. The AI summary mentions that prefixWorkspaceId was added to prefix the workspace_id field. However, the actual file on the PR branch shows workspace_id: workspaceId using the raw ID — no prefixWorkspaceId call. The AI summary appears to have been generated from a different commit state. The current code stores raw IDs in Tinybird, which is consistent with how queries against it would be written (see apiLogSchemaTB — the schema just expects a string).

apps/web/ui/layout/sidebar/app-sidebar-nav.tsx

Additive only. New "Logs" nav entry (StackY3 icon, /${slug}/settings/logs) added to the Developer section. Non-breaking UI addition.

apps/web/lib/api-logs/capture-webhook-log.ts

Whitespace only. No logic changes.


Breaking Changes

None for external API consumers. The internal function signature of captureRequestLog changed from sync to async, but all call sites across the codebase have been properly updated.


Pre-existing Issues (not introduced by this PR)

Two webhook routes call captureWebhookLog in a blocking manner unrelated to this PR:

  1. apps/web/app/(ee)/api/stripe/integration/webhook/route.ts (Line 163): Uses await captureWebhookLog(...) directly without waitUntil, which blocks the response.
  2. apps/web/app/(ee)/api/hubspot/webhook/route.ts (Line 92): Uses captureWebhookLog(...) inside a Promise.allSettled call without waitUntil.

Both should ideally wrap captureWebhookLog in waitUntil for consistency with the pattern established in this PR (and used in appsflyer and shopify webhook routes), but these are out of scope here.


Verdict

✅ Safe to merge. The refactoring is consistent and all call sites have been correctly updated. The workspace = undefined bug fix is a meaningful correctness improvement.

✅ Actions performed

Full review triggered.

@steven-tey steven-tey merged commit 0d3b0cc into main Apr 16, 2026
11 checks passed
@steven-tey steven-tey deleted the fix-logs branch April 16, 2026 20:50
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.

3 participants