Skip to content

fix: log HTTP response body on error in all integration clients - #66

Merged
chitcommit merged 2 commits into
mainfrom
fix/review-audit-findings
Mar 26, 2026
Merged

chitcommit merged 2 commits into
mainfrom
fix/review-audit-findings

Conversation

@chitcommit

@chitcommit chitcommit commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • All 16 !res.ok branches across integration clients now log the upstream response body (truncated to 500 chars)
  • Affected clients: ledger, evidence, finance, charge, connect, mercury, books, assets, scrape, router
  • Pattern: const body = await res.text().catch(() => ''); console.error(...)
  • Plaid and Notion already had this pattern — unchanged

Why

A 401 with {"error":"invalid token"} previously looked identical to a 500 or DNS failure in logs. This makes auth misconfigs, rate limits, and validation errors immediately diagnosable.

Test plan

  • npx tsc --noEmit passes (verified locally)
  • Deploy and trigger a known-bad token scenario — verify error body appears in logs
  • No functional behavior change — only logging improved

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling for ledger searches, chain of custody queries, and evidence fact retrieval with clearer error messages
    • Improved error logging and visibility across service integrations when requests fail
    • Better distinction between "no results" and "service unavailable" states for improved user experience

chitcommit and others added 2 commits March 26, 2026 00:58
…-catch

1. timeline.ts: apply branched SQL query pattern (matching mcp.ts fix)
2. mcp.ts get_case_timeline: wrap evidence fetch in try-catch, add
   warnings/partial fields for degraded responses
3. integrations.ts: searchEntries/getChainOfCustody now return
   ServiceArrayResult<T> with explicit error field so callers can
   distinguish "no data" from "service unreachable"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 16 `!res.ok` branches across ledger, evidence, finance, charge,
connect, mercury, books, assets, scrape, and router clients now capture
and log the response body (truncated to 500 chars). This surfaces
upstream error messages (auth failures, validation errors, rate limits)
that were previously silently dropped.

Plaid and Notion clients already had this pattern and were not changed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chitcommit
chitcommit merged commit d40d32c into main Mar 26, 2026
10 of 11 checks passed
@chitcommit
chitcommit deleted the fix/review-audit-findings branch March 26, 2026 01:52
@github-actions

Copy link
Copy Markdown
  1. @coderabbitai review
  2. @copilot review
  3. @codex review
  4. @claude review
    Adversarial review request: evaluate security, policy bypass paths, regression risk, and merge-gating bypass attempts.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b8b0e83-a487-45b0-9697-3d2e028223a9

📥 Commits

Reviewing files that changed from the base of the PR and between ad4c9e8 and 72f01b1.

📒 Files selected for processing (3)
  • src/lib/integrations.ts
  • src/routes/mcp.ts
  • src/routes/timeline.ts

📝 Walkthrough

Walkthrough

Changes improve error handling and response consistency across service clients. A new ServiceArrayResult interface standardizes array-returning API responses to distinguish "no results" from "service errors." Error logging is enhanced with response body inspection. Timeline query construction refactored for explicit date-range branching, and MCP timeline handler wraps facts retrieval in try-catch.

Changes

Cohort / File(s) Summary
Service Response Standardization
src/lib/integrations.ts
Introduced ServiceArrayResult<T> interface; updated ledgerClient.searchEntries() and ledgerClient.getChainOfCustody() to return ServiceArrayResult instead of bare arrays; returns { data: [], error: '...' } on upstream failure rather than empty array.
Enhanced Error Logging
src/lib/integrations.ts
Added response body inspection (via res.text() with fallback) across multiple client get/post paths (ledger, evidence, finance, charge, connect, mercury, books, assets, scrape, router); logs truncated body snippet (slice(0, 500)) and HTTP status on failure; some logs updated with method/path context; previously silent failures now emit console errors.
MCP Timeline Error Handling
src/routes/mcp.ts
Wrapped evidence.getFactsByDateRange() / evidence.getEnrichedFacts() calls in try-catch within get_case_timeline; on exception, logs error and pushes 'Evidence facts unavailable: service error' warning; existing "no facts" logic remains but moved inside try block.
Timeline Query Refactoring
src/routes/timeline.ts
Refactored deadline date-range filtering from parameterized inline conditionals to explicit branching: four separate SQL statements executed based on presence of startDate and/or endDate query parameters; selected fields, filtering, sorting, and event transformation logic unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through error logs with glee,
Catching stray failures, making responses clean—
Arrays wrapped in ServiceArrayResult dreams,
SQL branches bloom, and timelines gleam serene! ✨

✨ 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/review-audit-findings

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.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

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.

1 participant