Fix: Add missing stripe and resend error log sources#262
Conversation
The allowlist for the source parameter in GET /api/admin/error-logs and POST /api/admin/error-logs/batch-delete only included ["scraper", "email", "scheduler", "api"], but ErrorLogger writes logs with "stripe" and "resend" sources too. This meant those logs could not be filtered by source. Closes #260 https://claude.ai/code/session_01FfZuTLfQMSdpJZD78q8NXM
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaced inline error-log source allowlists with a shared Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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 `@server/routes.ts`:
- Around line 1435-1436: Create a shared constant and schema (e.g., export const
ERROR_LOG_SOURCES = ["scraper","email","scheduler","api","stripe","resend"] as
const; export const errorLogSourceSchema = z.enum(ERROR_LOG_SOURCES)) in the
shared module and import them via the `@shared/` alias; then replace every
hardcoded allowlist literal (the array used in the condition that checks
errorLogs.source and any other handlers that repeat the same array) with a
reference to ERROR_LOG_SOURCES for filtering and use errorLogSourceSchema to
validate/parse incoming source values before pushing conditions (i.e., replace
the inline ["scraper",...].includes(source) and similar checks with a
centralized import and validation).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 011a92ed-2376-49ef-ac74-dcb29a993842
📒 Files selected for processing (1)
server/routes.ts
server/routes.ts
Outdated
| if (source && ["scraper", "email", "scheduler", "api", "stripe", "resend"].includes(source)) { | ||
| conditions.push(eq(errorLogs.source, source)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Centralize error-log source allowlist to prevent repeat regressions
The fix works, but the same source allowlist is still hardcoded in multiple handlers. That duplication caused this exact bug and will drift again. Move allowed sources to a shared constant/schema (e.g., in shared/routes.ts) and reuse it in both filters.
Proposed refactor
- if (source && ["scraper", "email", "scheduler", "api", "stripe", "resend"].includes(source)) {
+ if (source && ERROR_LOG_SOURCES.includes(source as any)) {
conditions.push(eq(errorLogs.source, source));
}
...
- if (filters.source && ["scraper", "email", "scheduler", "api", "stripe", "resend"].includes(filters.source)) {
+ if (filters.source && ERROR_LOG_SOURCES.includes(filters.source as any)) {
conditions.push(eq(errorLogs.source, filters.source));
}// shared/routes.ts (or shared/constants/errorLogs.ts)
export const ERROR_LOG_SOURCES = [
"scraper",
"email",
"scheduler",
"api",
"stripe",
"resend",
] as const;
export const errorLogSourceSchema = z.enum(ERROR_LOG_SOURCES);As per coding guidelines: “All types, schemas, and constants shared between client and server must live in the shared/ directory and be imported using the @shared/ path alias. Never duplicate shared types in client or server code.”
Also applies to: 1556-1557
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes.ts` around lines 1435 - 1436, Create a shared constant and
schema (e.g., export const ERROR_LOG_SOURCES =
["scraper","email","scheduler","api","stripe","resend"] as const; export const
errorLogSourceSchema = z.enum(ERROR_LOG_SOURCES)) in the shared module and
import them via the `@shared/` alias; then replace every hardcoded allowlist
literal (the array used in the condition that checks errorLogs.source and any
other handlers that repeat the same array) with a reference to ERROR_LOG_SOURCES
for filtering and use errorLogSourceSchema to validate/parse incoming source
values before pushing conditions (i.e., replace the inline
["scraper",...].includes(source) and similar checks with a centralized import
and validation).
Move the hardcoded source filter array to a shared ERROR_LOG_SOURCES constant and import it in both admin error log endpoints. This prevents the allowlist from drifting when new sources are added. Addresses CodeRabbit review feedback on PR #262. https://claude.ai/code/session_01FfZuTLfQMSdpJZD78q8NXM
Dismissing stale bot review — fixes were pushed in subsequent commits.
Summary
"stripe"and"resend"to the source filter allowlist in bothGET /api/admin/error-logsandPOST /api/admin/error-logs/batch-deleteendpointsErrorLoggerbut could not be filtered in the admin panelTest plan
npm run checkpassesnpm run testpasses (1738 tests)source=stripereturns only stripe error logssource=resendreturns only resend error logsCloses #260
https://claude.ai/code/session_01FfZuTLfQMSdpJZD78q8NXM
Summary by CodeRabbit