Skip to content

refactor(shared): extract sanitizeApiError into shared helper - #4532

Merged
PierreBrisorgueil merged 2 commits into
masterfrom
refactor/4515-shared-api-error-sanitizer
Aug 3, 2026
Merged

refactor(shared): extract sanitizeApiError into shared helper#4532
PierreBrisorgueil merged 2 commits into
masterfrom
refactor/4515-shared-api-error-sanitizer

Conversation

@PierreBrisorgueil

Copy link
Copy Markdown
Collaborator

Summary

  • What changed: extracted the sanitizeApiError helper (previously duplicated byte-for-byte in admin.store.js and invitations.store.js) into a new shared module, src/lib/helpers/apiError.js, and imported it from both stores.
  • Why: two copies of a leak-prevention rule mean fixing one can leave the other leaking internal details (stack traces, DB paths) to end users.
  • Related issues: Closes 🔒 Share the API-error sanitiser instead of duplicating its redaction rule #4515

Scope

  • Modules impacted: lib/helpers (new), modules/admin/stores, modules/invitations/stores
  • Cross-module impact: none — pure extraction, no behavior change, no new dependency between stores
  • Risk level: low

Verbatim-copy contract

The function body (including the leak-prevention regex) is copied verbatim from both original call sites — confirmed byte-identical on baseline before extraction. Nothing in the redaction logic was changed or "improved": same 200-char cap, same internal-marker checks (collection/stack words, Error: prefix, stack frame, path segment, file-and-line reference), same generic fallback string.

Tests

New src/lib/helpers/tests/apiError.unit.tests.js — one test per redaction arm:

  • clean short message passes through
  • message with a stack frame → redacted
  • message mentioning an internal collection name → redacted
  • message over 200 characters → redacted
  • exactly 200 characters → allowed (boundary)
  • missing/absent message → generic fallback

Both admin.store and invitations.store unit suites stay green unchanged (they already exercised sanitizeApiError indirectly through store actions).

DoD grep grep -rn "const sanitizeApiError" src now only matches the helper's own export const declaration.

Validation

  • npm run lint
  • npm run test:unit
  • npm run build
  • Manual checks done (if applicable) — n/a, no UI/behavior change

Guardrails check

  • No secrets or credentials introduced (.env*, secrets/**, keys, tokens)
  • No risky rename/move of core stack paths
  • Changes remain merge-friendly for downstream projects
  • Tests added or updated when behavior changed

Notes for reviewers

  • Security considerations: this refactor centralizes a leak-prevention control (regex that strips stack traces/internal paths from user-facing error messages). Behavior is unchanged; a future fix now only needs to touch one file instead of two.
  • Mergeability considerations: this PR blocks 🔧 Single source for the API base URL #4516 (also edits admin.store.js and invitations.store.js on adjacent lines per that issue) — land this first, then rebase 🔧 Single source for the API base URL #4516.
  • Follow-up tasks (optional): none

sanitizeApiError was duplicated byte-for-byte in admin.store.js and
invitations.store.js, including the leak-prevention regex that strips
stack traces and internal paths from user-facing error messages. Two
copies of a redaction rule means fixing one can leave the other
leaking.

Move it to src/lib/helpers/apiError.js (verbatim, same contract) and
import it from both stores.

Closes #4515
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@PierreBrisorgueil, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ebd6581f-b72a-4915-89cd-110b9e1692d3

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab89c4 and 963b293.

📒 Files selected for processing (4)
  • src/lib/helpers/apiError.js
  • src/lib/helpers/tests/apiError.unit.tests.js
  • src/modules/admin/stores/admin.store.js
  • src/modules/invitations/stores/invitations.store.js

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.

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.57%. Comparing base (1ab89c4) to head (963b293).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4532      +/-   ##
==========================================
- Coverage   99.57%   99.57%   -0.01%     
==========================================
  Files          36       37       +1     
  Lines        1427     1422       -5     
  Branches      448      446       -2     
==========================================
- Hits         1421     1416       -5     
  Misses          6        6              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@PierreBrisorgueil
PierreBrisorgueil marked this pull request as ready for review August 3, 2026 11:58
@PierreBrisorgueil

Copy link
Copy Markdown
Collaborator Author

CodeRabbit hit its Pro review-limit (58 min cooldown reported at PR-open time), so per /dev:pull-request-finalize's documented fallback this PR was independently reviewed by a cold-context Claude reviewer (sonnet) in addition to the pre-push kimi gate.

Both independent reviewers returned 0 findings / OK:

  • Pre-push gate (kimi-k2.7-code, local diff): OK — 0 findings
  • Fallback reviewer (claude, cold-context, PR refactor(shared): extract sanitizeApiError into shared helper #4532 diff): OK — 0 findings, confirmed the extracted sanitizeApiError is byte-for-byte identical to both removed copies (same regex, same length gate, same fallback string)

CI green: Lint/Unit Tests/Build, E2E Tests, codecov patch + project.

@PierreBrisorgueil
PierreBrisorgueil merged commit 3e4a068 into master Aug 3, 2026
7 checks passed
@PierreBrisorgueil
PierreBrisorgueil deleted the refactor/4515-shared-api-error-sanitizer branch August 3, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🔒 Share the API-error sanitiser instead of duplicating its redaction rule

1 participant