feat: add api_errors table and service for Sentry error aggregation - #811
Merged
Conversation
…se 1) Adds the aggregated `api_errors` table and `apiErrorService` defined in Epic #616 Phase 1 / sub-issue #802. Raw stack traces stay in Sentry; this table only stores per-issue state (occurrences, severity, status, AI analysis output, GitHub issue mapping) keyed on `sentry_issue_id`. - Schema: `server/api/src/schema/apiErrors.ts` with severity / status enums and indexes for the admin error page hot paths. - Migration: `0019_add_api_errors.sql` + `_journal.json` entry. - Service: `apiErrorService.upsertFromSentrySummary` increments `occurrences` and advances `last_seen_at` (GREATEST) while preserving `first_seen_at`; list / get / status-update helpers; status transition validation rejects illegal jumps such as `ignored -> resolved`. - Tests: 25 Vitest cases covering transition rules, the upsert contract (initial insert + recurrence with occurrences increment and first_seen_at preservation), list filtering, single-row lookups, and status update with invalid / same-state / not-found branches. Refs #616 Closes #802
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new Postgres migration and Drizzle schema for an api_errors table, implements an apiErrorService (upsert from Sentry summaries, list/get, status transitions with optimistic concurrency), re-exports schema types, and adds comprehensive Vitest coverage for the service. ChangesAPI Error Tracking Feature
Sequence Diagram(s)sequenceDiagram
participant Sentry as Sentry Webhook
participant Service as apiErrorService
participant DB as PostgreSQL (api_errors)
participant Admin as Admin UI
Sentry->>Service: POST summary (sentryIssueId, title, fingerprint, route, status_code, occurrencesDelta, severity)
Service->>DB: INSERT ... ON CONFLICT (sentry_issue_id) DO UPDATE (increment occurrences, GREATEST(last_seen_at), COALESCE fields, preserve first_seen_at)
DB-->>Service: upserted row
Service-->>Sentry: ack/200
Admin->>Service: GET listApiErrors(filters, limit, offset)
Service->>DB: SELECT rows + COUNT with filters/order
DB-->>Service: rows + total
Service-->>Admin: rows + total
Admin->>Service: POST updateApiErrorStatus(id, nextStatus)
Service->>DB: conditional UPDATE WHERE id = ? AND status = previousStatus
DB-->>Service: updated row or zero rows
alt zero rows
Service-->>Admin: throws ApiErrorStatusConflictError
else
Service-->>Admin: updated row
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 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)
Review rate limit: 3/5 reviews remaining, refill in 19 minutes and 57 seconds. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the api_errors table and a service layer to aggregate Sentry-detected API errors, including schema definitions, migrations, and unit tests. The service handles idempotent upserts and workflow status management. Feedback was provided to improve input validation in the upsertFromSentrySummary function, specifically to handle null inputs, ensure required fields like title are not empty, and guarantee numeric safety for occurrence increments.
Comment on lines
+134
to
+144
| const sentryIssueId = input.sentryIssueId.trim(); | ||
| if (!sentryIssueId) { | ||
| throw new Error("sentryIssueId is required"); | ||
| } | ||
| const occurrencesDelta = Math.max(1, Math.floor(input.occurrencesDelta ?? 1)); | ||
| const now = input.lastSeenAt ?? new Date(); | ||
|
|
||
| const values: NewApiError = { | ||
| sentryIssueId, | ||
| fingerprint: input.fingerprint ?? null, | ||
| title: input.title, |
There was a problem hiding this comment.
The input validation in upsertFromSentrySummary should be more robust to handle edge cases from external webhook data:
- Defensive validation: Use nullish coalescing before calling
.trim()onsentryIssueIdto avoid a potentialTypeErrorif the input is unexpectedly null or undefined at runtime. - Title validation: Since
titleis a required non-null column in the database, it should be trimmed and validated for emptiness to ensure data integrity. - Numeric safety: Ensure
occurrencesDeltais a finite number. IfNaNis passed (e.g., from a malformed payload), the currentMath.maxlogic will returnNaN, which will cause the database query to fail.
Suggested change
| const sentryIssueId = input.sentryIssueId.trim(); | |
| if (!sentryIssueId) { | |
| throw new Error("sentryIssueId is required"); | |
| } | |
| const occurrencesDelta = Math.max(1, Math.floor(input.occurrencesDelta ?? 1)); | |
| const now = input.lastSeenAt ?? new Date(); | |
| const values: NewApiError = { | |
| sentryIssueId, | |
| fingerprint: input.fingerprint ?? null, | |
| title: input.title, | |
| const sentryIssueId = (input.sentryIssueId ?? "").trim(); | |
| if (!sentryIssueId) { | |
| throw new Error("sentryIssueId is required"); | |
| } | |
| const title = (input.title ?? "").trim(); | |
| if (!title) { | |
| throw new Error("title is required"); | |
| } | |
| const delta = input.occurrencesDelta ?? 1; | |
| const occurrencesDelta = Number.isFinite(delta) ? Math.max(1, Math.floor(delta)) : 1; | |
| const now = input.lastSeenAt ?? new Date(); | |
| const values: NewApiError = { | |
| sentryIssueId, | |
| fingerprint: input.fingerprint ?? null, | |
| title, |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 809d82a688
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+157
to
+177
| .onConflictDoUpdate({ | ||
| target: apiErrors.sentryIssueId, | ||
| set: { | ||
| // occurrences は EXCLUDED 値（= occurrencesDelta）で加算する。 | ||
| // Increment by EXCLUDED.occurrences (== occurrencesDelta on this call). | ||
| occurrences: sql`${apiErrors.occurrences} + EXCLUDED.occurrences`, | ||
| // last_seen_at は新旧のうち遅い方を採用する。 | ||
| // last_seen_at advances to whichever timestamp is later. | ||
| lastSeenAt: sql`GREATEST(${apiErrors.lastSeenAt}, EXCLUDED.last_seen_at)`, | ||
| // 表示用カラムは新しい値を採用、null の場合は既存を維持。 | ||
| // Descriptive columns refresh from EXCLUDED, preserving on null. | ||
| title: sql`EXCLUDED.title`, | ||
| route: sql`COALESCE(EXCLUDED.route, ${apiErrors.route})`, | ||
| statusCode: sql`COALESCE(EXCLUDED.status_code, ${apiErrors.statusCode})`, | ||
| fingerprint: sql`COALESCE(EXCLUDED.fingerprint, ${apiErrors.fingerprint})`, | ||
| updatedAt: sql`NOW()`, | ||
| // first_seen_at / status / severity / ai_* / github_issue_number は意図的に | ||
| // 触らない。再来で初回時刻や人手で更新した状態を巻き戻さないため。 | ||
| // first_seen_at, status, severity, ai_*, github_issue_number are | ||
| // intentionally untouched: a re-occurrence must not rewind first-seen | ||
| // or undo human / AI updates. |
There was a problem hiding this comment.
In the conflict-update branch of upsertFromSentrySummary, the row’s status is intentionally left untouched, so a previously resolved issue remains resolved even when new occurrences arrive. This conflicts with the declared workflow (resolved should reopen on regression) and can hide active regressions from the admin’s default unresolved/error-triage views, because only occurrences/last_seen_at advance while status stays closed.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/api/drizzle/0019_add_api_errors.sql`:
- Around line 27-28: The migration currently allows any string for the "status"
and "severity" columns; add strong DB-level constraints (create Postgres ENUM
types or CHECK constraints) and change the column definitions to use them (e.g.,
CREATE TYPE api_error_status AS ENUM(...); "status" api_error_status NOT NULL
DEFAULT 'open', and similarly for "severity") so runtime writes cannot slip
outside the allowed workflow, then update the Drizzle schema for the apiErrors
table to use the same enum types/values so it stays aligned with the DB and
keeps listApiErrors and updateApiErrorStatus operating on the same enforced set
of values.
In `@server/api/src/services/apiErrorService.ts`:
- Around line 302-317: The current read-validate-update in update flow using
getApiErrorById and assertValidApiErrorStatusTransition is vulnerable to races;
make the change atomic by either wrapping the read+validation+write in a
transaction or by including the previously-read status in the UPDATE predicate
(e.g. WHERE apiErrors.id = input.id AND apiErrors.status = current.status) and
then treat a 0-rows-updated result as a concurrent modification error; use
apiErrors, db.update(...).set(...) and the same status value returned from
getApiErrorById to gate the write and return a clear concurrency failure when no
rows are updated.
🪄 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: CHILL
Plan: Pro
Run ID: 1a7b82f3-941a-4eb5-ad5c-84d8a92c22bd
📒 Files selected for processing (6)
server/api/drizzle/0019_add_api_errors.sqlserver/api/drizzle/meta/_journal.jsonserver/api/src/schema/apiErrors.tsserver/api/src/schema/index.tsserver/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.ts
Addresses PR #811 review feedback: - upsertFromSentrySummary now defensively validates external webhook input (null-safe sentry_issue_id and title, finite occurrencesDelta) before building the insert payload. - On recurrence the upsert reopens rows whose status was 'resolved' so regressions surface in the admin's open view; 'ignored' stays put to respect the explicit admin decision. - updateApiErrorStatus is now race-free: the UPDATE WHERE clause pins on the previously-read status, and a 0-rows result throws the new ApiErrorStatusConflictError (caller maps to HTTP 409). - Migration adds CHECK constraints for status/severity so the DB rejects values outside the workflow; the Drizzle column types use enum literals to mirror the constraint at the type level. - Tests grow to 30 cases covering the new validation, recurrence reopen, and concurrent-modification branches.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/api/src/services/apiErrorService.test.ts (2)
249-276: ⚡ Quick winAdd the “ignored stays ignored” recurrence case.
The service contract here is two-sided: recurrence should reopen
resolved, but it must not reopenignored. This test only protects the first branch, so a future change to theCASEexpression could regress the second one without failing the suite.As per coding guidelines, "Tests serve as a source of truth for specifications alongside implementation code TSDoc/JSDoc."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/src/services/apiErrorService.test.ts` around lines 249 - 276, Add a new unit test mirroring the existing "reopens to status='open'" test but asserting that a previously-ignored row remains ignored on recurrence: create a mocked post-upsert row via makeRow with status: "ignored", appropriate occurrences/firstSeenAt/lastSeenAt, use createMockDb to return that row, call upsertFromSentrySummary with occurrencesDelta > 0 and the same sentryIssueId/title, then assert result.status === "ignored" and that firstSeenAt is unchanged (e.g., toISOString matches the original); place the test alongside the existing one to protect the "ignored stays ignored" branch of the upsert CASE expression.
61-70: ⚡ Quick winThis mock makes the query-shape assertions vacuous.
Because every builder method ignores its arguments and resolves the same canned result, tests like the list/filter/clamp cases still pass even if
.where(),.limit(), or.offset()disappear fromlistApiErrors. Please capture the invoked methods/args or use a stub that asserts the expected builder calls so these tests actually lock in the query contract.As per coding guidelines, "Tests serve as a source of truth for specifications alongside implementation code TSDoc/JSDoc."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/src/services/apiErrorService.test.ts` around lines 61 - 70, The current createMockDb returns a Proxy where every builder method ignores its name/arguments and always returns the same canned makeChainProxy result, making assertions on .where/.limit/.offset vacuous; update createMockDb to record each invoked method name and its arguments (e.g., push {method, args} into a calls array) and/or allow per-call responses keyed by expected method sequence so tests can assert the exact builder calls; reference createMockDb and makeChainProxy in the change and ensure the tests (e.g., list/filter/clamp cases) assert the recorded calls array contains the expected method names and argument shapes rather than relying on the canned result.
🤖 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/api/src/services/apiErrorService.ts`:
- Around line 146-159: Sanitize and validate statusCode and timestamp fields at
the same boundary where occurrencesDelta/rawDelta and now are computed: coerce
input.statusCode into a finite integer (e.g., Number(input.statusCode), check
Number.isFinite, Math.floor, and constrain to valid HTTP range or null) and
parse input.firstSeenAt and input.lastSeenAt into Date objects (fall back to the
computed now for invalid or missing values and ensure firstSeenAt ≤ lastSeenAt),
then use these sanitized values when building the NewApiError object
(referencing occurrencesDelta, rawDelta, now, statusCode, firstSeenAt,
lastSeenAt, and NewApiError) so no NaN/Infinity/fractional or Invalid Date
values reach the DB.
---
Nitpick comments:
In `@server/api/src/services/apiErrorService.test.ts`:
- Around line 249-276: Add a new unit test mirroring the existing "reopens to
status='open'" test but asserting that a previously-ignored row remains ignored
on recurrence: create a mocked post-upsert row via makeRow with status:
"ignored", appropriate occurrences/firstSeenAt/lastSeenAt, use createMockDb to
return that row, call upsertFromSentrySummary with occurrencesDelta > 0 and the
same sentryIssueId/title, then assert result.status === "ignored" and that
firstSeenAt is unchanged (e.g., toISOString matches the original); place the
test alongside the existing one to protect the "ignored stays ignored" branch of
the upsert CASE expression.
- Around line 61-70: The current createMockDb returns a Proxy where every
builder method ignores its name/arguments and always returns the same canned
makeChainProxy result, making assertions on .where/.limit/.offset vacuous;
update createMockDb to record each invoked method name and its arguments (e.g.,
push {method, args} into a calls array) and/or allow per-call responses keyed by
expected method sequence so tests can assert the exact builder calls; reference
createMockDb and makeChainProxy in the change and ensure the tests (e.g.,
list/filter/clamp cases) assert the recorded calls array contains the expected
method names and argument shapes rather than relying on the canned result.
🪄 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: CHILL
Plan: Pro
Run ID: 3767274d-c970-446c-9f65-67f8e59d0550
📒 Files selected for processing (4)
server/api/drizzle/0019_add_api_errors.sqlserver/api/src/schema/apiErrors.tsserver/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.ts
✅ Files skipped from review due to trivial changes (2)
- server/api/drizzle/0019_add_api_errors.sql
- server/api/src/schema/apiErrors.ts
Hardens the webhook entry point with two more boundary checks and adds the symmetric "ignored stays ignored" recurrence test, addressing the follow-up CodeRabbit review on #811. - statusCode that is not a finite integer (NaN, Infinity, fractional) now throws "statusCode must be a finite integer". - firstSeenAt / lastSeenAt that resolve to Invalid Date throw "firstSeenAt|lastSeenAt must be a valid Date". - Validation extracted into normalizeUpsertInput so the public upsertFromSentrySummary stays under the cyclomatic-complexity ceiling. - New test asserts that a recurrence on a row whose status is "ignored" preserves the ignored decision (locks down the CASE expression's ELSE branch, complementing the resolved → open test).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/api/src/services/apiErrorService.ts (1)
114-141: ⚡ Quick winAttach TSDoc directly to exported
upsertFromSentrySummary.
upsertFromSentrySummaryis exported at Line 174, but the nearest attached doc block is fornormalizeUpsertInput(Line 134 onward). Please move/add a TSDoc block immediately above Line 174 so the exported API is documented at the declaration site.As per coding guidelines
**/*.{ts,tsx}: Add TSDoc / JSDoc comments to exported functions, types, and interfaces.Also applies to: 174-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/src/services/apiErrorService.ts` around lines 114 - 141, The exported function upsertFromSentrySummary currently lacks a TSDoc block at its declaration; move or add the detailed TSDoc/JSDoc that describes parameters, return, and thrown errors so it sits immediately above the exported function declaration (upsertFromSentrySummary) rather than above normalizeUpsertInput, and apply the same pattern to any other exported symbols listed around 174–177 so all exported functions/types in this file have a TSDoc block directly attached to their declarations.
🤖 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/api/src/services/apiErrorService.ts`:
- Around line 275-280: The current computation of limit and offset uses
Number(...) and clamp(...) which preserves fractional values; before calling
clamp, parse and coerce filters.limit and filters.offset to integers (e.g., use
parseInt or Math.floor on Number(filters.limit ?? API_ERROR_LIST_DEFAULT_LIMIT)
and Number(filters.offset ?? 0)) so that the variables limit and offset are
always integers; update the expressions that set limit and offset (the variables
computed with clamp) to apply the integer coercion first, keeping the same
bounds API_ERROR_LIST_DEFAULT_LIMIT, API_ERROR_LIST_MAX_LIMIT, and
Number.MAX_SAFE_INTEGER.
---
Nitpick comments:
In `@server/api/src/services/apiErrorService.ts`:
- Around line 114-141: The exported function upsertFromSentrySummary currently
lacks a TSDoc block at its declaration; move or add the detailed TSDoc/JSDoc
that describes parameters, return, and thrown errors so it sits immediately
above the exported function declaration (upsertFromSentrySummary) rather than
above normalizeUpsertInput, and apply the same pattern to any other exported
symbols listed around 174–177 so all exported functions/types in this file have
a TSDoc block directly attached to their declarations.
🪄 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: CHILL
Plan: Pro
Run ID: 3f7c924a-eb75-4553-acb4-f344a65a5d33
📒 Files selected for processing (2)
server/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/api/src/services/apiErrorService.test.ts
Two CodeRabbit follow-ups on PR #811: - listApiErrors now floors limit/offset before clamping. A fractional or NaN value from a query string would otherwise reach Postgres as a non-integer LIMIT/OFFSET and fail at runtime; non-finite inputs fall back to the documented defaults. Adds a regression test for fractional and NaN/Infinity inputs. - Restored the upsertFromSentrySummary TSDoc directly above its declaration. The earlier extraction of normalizeUpsertInput inserted the helper between the public function and its docs, which left the exported API undocumented at the declaration site.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/api/src/services/apiErrorService.ts (1)
142-150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
firstSeenAtandlastSeenAtmonotonic.These checks prove each timestamp is a valid
Date, but they still allowfirstSeenAt > lastSeenAt. That lets a malformed webhook persist an inverted timeline intoapi_errors, which breaks the core first/last-seen invariant for new rows.Suggested fix
function normalizeUpsertInput(input: UpsertFromSentrySummaryInput): { sentryIssueId: string; title: string; occurrencesDelta: number; + firstSeenAt: Date; now: Date; } { const sentryIssueId = (input.sentryIssueId ?? "").trim(); if (!sentryIssueId) { throw new Error("sentryIssueId is required"); @@ } const rawDelta = input.occurrencesDelta ?? 1; const occurrencesDelta = Number.isFinite(rawDelta) ? Math.max(1, Math.floor(rawDelta)) : 1; const now = input.lastSeenAt ?? new Date(); - return { sentryIssueId, title, occurrencesDelta, now }; + const firstSeenAt = input.firstSeenAt ?? now; + if (firstSeenAt.getTime() > now.getTime()) { + throw new Error("firstSeenAt must be less than or equal to lastSeenAt"); + } + return { sentryIssueId, title, occurrencesDelta, firstSeenAt, now }; } @@ - const { sentryIssueId, title, occurrencesDelta, now } = normalizeUpsertInput(input); + const { sentryIssueId, title, occurrencesDelta, firstSeenAt, now } = + normalizeUpsertInput(input); @@ - firstSeenAt: input.firstSeenAt ?? now, + firstSeenAt,Also applies to: 181-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/src/services/apiErrorService.ts` around lines 142 - 150, The validation allows invalid timelines because it only checks Date validity but not ordering; update the logic in apiErrorService.ts to enforce monotonicity by verifying that when both input.firstSeenAt and input.lastSeenAt are provided then input.firstSeenAt.getTime() <= input.lastSeenAt.getTime(), and if not either throw a descriptive Error (e.g., "firstSeenAt must be <= lastSeenAt") or normalize (choose policy) before computing rawDelta/occurrencesDelta and setting now; apply the same monotonic check to the other block around the 181-190 range that mirrors this logic so new rows cannot be created with firstSeenAt > lastSeenAt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/api/src/services/apiErrorService.ts`:
- Around line 142-150: The validation allows invalid timelines because it only
checks Date validity but not ordering; update the logic in apiErrorService.ts to
enforce monotonicity by verifying that when both input.firstSeenAt and
input.lastSeenAt are provided then input.firstSeenAt.getTime() <=
input.lastSeenAt.getTime(), and if not either throw a descriptive Error (e.g.,
"firstSeenAt must be <= lastSeenAt") or normalize (choose policy) before
computing rawDelta/occurrencesDelta and setting now; apply the same monotonic
check to the other block around the 181-190 range that mirrors this logic so new
rows cannot be created with firstSeenAt > lastSeenAt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ad3bb2c-d4d7-429e-8952-f11cf799d7d6
📒 Files selected for processing (2)
server/api/src/services/apiErrorService.test.tsserver/api/src/services/apiErrorService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/api/src/services/apiErrorService.test.ts
Addresses CodeRabbit follow-up on PR #811: a webhook supplying firstSeenAt > lastSeenAt could persist an inverted timeline into api_errors and silently break the first/last-seen invariant. The helper now throws "firstSeenAt must be less than or equal to lastSeenAt" when the two timestamps disagree, and threads the normalized firstSeenAt through to the insert payload so the public function relies on a single source of truth. Adds tests for the inverted-timeline rejection and the equal-timestamps boundary.
This was referenced May 4, 2026
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Sentry Webhook ハンドラと管理者画面向けの API エラー集約機能を実装します。
api_errorsテーブルを追加し、Sentry が検知したエラーをsentry_issue_idをキーに重複排除・集約します。発生回数の加算、初回観測時刻の保持、ワークフロー状態管理（open ↔ investigating ↔ resolved/ignored）を提供します。変更点
server/api/src/schema/apiErrors.ts(新規)api_errorsテーブルの Drizzle スキーマ定義ApiErrorStatus(open/investigating/resolved/ignored) とApiErrorSeverity(high/medium/low/unknown) の型定義ApiErrorSuspectedFileインターフェース（AI 推定ファイル）server/api/src/services/apiErrorService.ts(新規)ALLOWED_API_ERROR_STATUS_TRANSITIONS: 状態遷移ルール（ignored → resolved 直接遷移禁止）isValidApiErrorStatusTransition()/assertValidApiErrorStatusTransition(): 遷移検証upsertFromSentrySummary(): Sentry Webhook 用 upsert（occurrences 加算、first_seen_at 保持）listApiErrors(): フィルタ・ページネーション対応の一覧取得getApiErrorById()/getApiErrorBySentryIssueId(): 単件取得updateApiErrorStatus(): ワークフロー状態更新（遷移検証付き）server/api/src/services/apiErrorService.test.ts(新規)server/api/drizzle/0019_add_api_errors.sql(新規)api_errorsテーブル作成 SQLsentry_issue_idユニーク制約server/api/src/schema/index.ts(修正)apiErrorsテーブルと関連型をエクスポートserver/api/drizzle/meta/_journal.json(修正)0019_add_api_errorsを追加変更の種類
テスト方法
apiErrorService.test.tsで状態遷移・upsert・一覧取得・単件取得・状態更新をカバーnpm run testで全テストが通ることを確認チェックリスト
https://claude.ai/code/session_01Uu5RKkNr23MJGqsLjohR8e
Summary by CodeRabbit
New Features
Tests
Chores