feat: Phase 30 Web App Observability#386
Merged
Merged
Conversation
Entire-Checkpoint: 41c78a918d41
Entire-Checkpoint: f1d4ff1e41f5
Entire-Checkpoint: 392c6e215102
…ed asterisks Entire-Checkpoint: f3427deb3f7a
Entire-Checkpoint: 991c95ee1b08
Four plans in 2 waves: - 28-01: Create logger module with level filtering, redaction, transport hooks - 28-02: Replace console.* in 8 high-volume files (97 calls) - 28-03: Replace console.* in 21 remaining files + fix 11 silenced unpin catches - 28-04: Fix as-any casts, delete legacy POC, add ESLint no-console rule Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 0f0c86c5c938
Entire-Checkpoint: c0efcb8660ff
Three plans in 2 waves: - 29-01: IPNS batch unenroll API endpoint (Wave 1) - 29-02: SDK unenrollment on delete + legacy TODO cleanup (Wave 2) - 29-03: Test login monitoring alert + Kubo access verification (Wave 1) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 3221a59f2435
Entire-Checkpoint: 5b395217989c
Four plans covering Grafana Faro SDK integration with strict privacy scrubbing, error boundary, source map upload, and logger transport wiring. Wave 1: SDK init + privacy. Wave 2: error boundary, deploy config, and logger integration (parallel). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a994dec397b1
Install @grafana/faro-web-sdk and @grafana/faro-react, create the Faro initialization module with strict beforeSend privacy gate that strips all sensitive CipherBox fields (keys, tokens, emails) and hex-encoded key patterns before any telemetry leaves the browser. Wire initFaro() into main.tsx before React render tree. Faro is completely disabled when VITE_FARO_URL is absent (local dev). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wrap the route tree with FaroErrorBoundary so unhandled React render errors are captured by Faro and users see a recovery screen instead of a white page. The fallback reassures users their encrypted data is safe and provides a reload button. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add @grafana/faro-rollup-plugin to upload source maps to Grafana Cloud at build time (production/staging only) so minified stack traces are readable. Source maps use 'hidden' mode -- generated for upload but not served to the browser. Add VITE_FARO_URL, GRAFANA_FARO_API_KEY, and GRAFANA_STACK_ID to all build steps in deploy-staging.yml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire setFaroUser(publicKey) into the auth success path and session restoration so error reports are associated with users (publicKey only, never email). Clear Faro user on logout to prevent post-logout errors from being attributed. Also export registerFaroTransport for Phase 28 logger integration and clearFaroUser for logout cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All 4 plans verified: Faro SDK with privacy scrubbing, error boundary with terminal-aesthetic fallback, source map upload configuration, and user identity binding on auth. Phase goal achieved -- errors and performance issues in the deployed web app are now capturable and trackable via Grafana Faro. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add string length early-exit before hex regex in scrubValue - Skip object spread in beforeSend when nothing changed - Extract shared toStringContext helper for transport - Simplify clearFaroUser type cast - Condense verbose JSDoc comments - Remove unnecessary WHAT comments in useAuth/main Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 630bd01f676c
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds Grafana Faro observability to the web app and staging builds: Faro SDK init with privacy scrubbing and React error boundary, Faro rollup plugin and source-map upload wiring, CI env injection for Faro, Faro auth hooks, and related planning/docs. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds privacy-focused client-side observability to the web app using Grafana Faro, including a React error boundary fallback UI, source map upload wiring for staging/production builds, and user identity binding (publicKey-only) during auth/logout.
Changes:
- Integrate Grafana Faro SDK + React integration with a
beforeSendscrubbing gate and auth-driven user identity binding. - Add
FaroErrorBoundarywrapper and a terminal-styled error fallback UI. - Configure hidden sourcemaps + Faro rollup plugin upload and wire required env vars into the staging deploy workflow.
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds Faro SDK and rollup plugin dependencies and transitive OTel/web-vitals deps. |
| package.json | Adjusts lint-staged markdownlint invocation to ignore .planning. |
| apps/web/vite.config.ts | Adds Faro rollup plugin config and enables hidden sourcemaps. |
| apps/web/src/vite-env.d.ts | Adds Vite env typings for Faro URL and app version. |
| apps/web/src/main.tsx | Initializes Faro before React renders. |
| apps/web/src/lib/faro.ts | Implements Faro initialization, beforeSend privacy scrubbing, user binding, and logger transport hook. |
| apps/web/src/hooks/useAuth.ts | Binds/clears Faro user identity on auth/session restore/logout. |
| apps/web/src/components/ErrorFallback.tsx | Adds error boundary fallback component with reload action. |
| apps/web/src/App.tsx | Wraps routes with FaroErrorBoundary. |
| apps/web/src/App.css | Adds styling for the error fallback UI. |
| apps/web/package.json | Adds Faro SDK dependencies and rollup plugin devDependency. |
| .planning/phases/30-web-app-observability/30-VERIFICATION.md | Adds Phase 30 verification record. |
| .planning/phases/30-web-app-observability/30-DISCUSSION-LOG.md | Adds Phase 30 discussion log. |
| .planning/phases/30-web-app-observability/30-CONTEXT.md | Adds Phase 30 context and decisions. |
| .planning/phases/30-web-app-observability/30-04-SUMMARY.md | Adds summary for user identity binding + transport readiness. |
| .planning/phases/30-web-app-observability/30-04-PLAN.md | Adds plan for transport + identity binding. |
| .planning/phases/30-web-app-observability/30-03-SUMMARY.md | Adds summary for sourcemap upload + staging env wiring. |
| .planning/phases/30-web-app-observability/30-03-PLAN.md | Adds plan for sourcemap upload + staging env wiring. |
| .planning/phases/30-web-app-observability/30-02-SUMMARY.md | Adds summary for error boundary + fallback UI. |
| .planning/phases/30-web-app-observability/30-02-PLAN.md | Adds plan for error boundary + fallback UI. |
| .planning/phases/30-web-app-observability/30-01-SUMMARY.md | Adds summary for Faro init + privacy gate. |
| .planning/phases/30-web-app-observability/30-01-PLAN.md | Adds plan for Faro init + privacy gate. |
| .planning/phases/29-infrastructure-hardening/29-DISCUSSION-LOG.md | Adds Phase 29 assumptions discussion log. |
| .planning/phases/29-infrastructure-hardening/29-CONTEXT.md | Adds Phase 29 context and decisions. |
| .planning/phases/29-infrastructure-hardening/29-03-PLAN.md | Adds plan for test-login alert + Kubo verification. |
| .planning/phases/29-infrastructure-hardening/29-02-PLAN.md | Adds plan for SDK-side IPNS unenrollment on delete. |
| .planning/phases/29-infrastructure-hardening/29-01-PLAN.md | Adds plan for API batch IPNS unenroll endpoint + client regen. |
| .planning/phases/28-code-hygiene-logging/28-DISCUSSION-LOG.md | Adds Phase 28 assumptions discussion log. |
| .planning/phases/28-code-hygiene-logging/28-CONTEXT.md | Adds Phase 28 context and decisions. |
| .planning/phases/28-code-hygiene-logging/28-04-PLAN.md | Adds plan for any-cast cleanup, POC deletion, and no-console rule. |
| .planning/phases/28-code-hygiene-logging/28-03-PLAN.md | Adds plan for console replacement + unpin catch visibility. |
| .planning/phases/28-code-hygiene-logging/28-02-PLAN.md | Adds plan for high-volume console replacement. |
| .planning/phases/28-code-hygiene-logging/28-01-PLAN.md | Adds plan for logger module creation. |
| .planning/STATE.md | Updates planning state/progress metadata and current focus. |
| .planning/ROADMAP.md | Updates roadmap status markers and Phase 30 completion entry. |
| .markdownlintignore | Ignores the full .planning/ directory. |
| .github/workflows/deploy-staging.yml | Adds Faro-related env vars/secrets to all staging build steps. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
…rvability # Conflicts: # .planning/ROADMAP.md # .planning/STATE.md # .planning/phases/28-code-hygiene-logging/28-01-PLAN.md # .planning/phases/28-code-hygiene-logging/28-02-PLAN.md # .planning/phases/28-code-hygiene-logging/28-03-PLAN.md # .planning/phases/28-code-hygiene-logging/28-04-PLAN.md
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/src/lib/faro.ts`:
- Around line 38-40: The scrubObject function currently returns the original
subtree when depth > 3 and never descends into arrays, allowing sensitive values
(including Uint8Array) to leak; update scrubObject to perform a cycle-safe
recursive walk (use a WeakSet/WeakMap to track seen objects) that recurses into
objects and arrays, redacts or replaces the entire subtree once the max depth is
reached instead of returning it, and explicitly handle Uint8Array and sensitive
keys during traversal; also apply the same fix for the related logic referenced
around the other scrub block (the code at the second occurrence) so both places
use the same cycle-safe recursion and subtree-redaction behavior.
- Around line 11-20: Add the generic "email" key to the SENSITIVE_KEYS set and
update the redaction logic that currently targets only "userEmail" and
"meta.user.email" so it also strips any field named "email" at top-level and
within nested objects; specifically modify the SENSITIVE_KEYS constant (add
'email') and adjust the sanitization code that references "userEmail" /
"meta.user.email" to check for keys in SENSITIVE_KEYS (or explicitly check for
'email') when iterating object keys so all "email" fields are redacted
consistently (also apply the same change where similar sets/logic appear around
the other referenced locations).
- Around line 151-156: The helper toStringContext currently eagerly calls
String(v) for every non-error key (function toStringContext), which serializes
binary and object values before redaction; change it so it only coerces actual
string values and preserves other value types (leave Uint8Array, objects, etc.
as-is) so redaction/scrubbing in beforeSend can operate on structured data;
apply the same fix to the similar conversion code around the other helper at
lines 163-171 so both keep non-string values intact instead of converting them
to comma-separated bytes or “[object Object]”.
🪄 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: 8021c6de-09ff-4030-b325-27df3ee5ef47
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (36)
.github/workflows/deploy-staging.yml.markdownlintignore.planning/ROADMAP.md.planning/STATE.md.planning/phases/28-code-hygiene-logging/28-01-PLAN.md.planning/phases/28-code-hygiene-logging/28-02-PLAN.md.planning/phases/28-code-hygiene-logging/28-03-PLAN.md.planning/phases/28-code-hygiene-logging/28-04-PLAN.md.planning/phases/28-code-hygiene-logging/28-CONTEXT.md.planning/phases/28-code-hygiene-logging/28-DISCUSSION-LOG.md.planning/phases/29-infrastructure-hardening/29-01-PLAN.md.planning/phases/29-infrastructure-hardening/29-02-PLAN.md.planning/phases/29-infrastructure-hardening/29-03-PLAN.md.planning/phases/29-infrastructure-hardening/29-CONTEXT.md.planning/phases/29-infrastructure-hardening/29-DISCUSSION-LOG.md.planning/phases/30-web-app-observability/30-01-PLAN.md.planning/phases/30-web-app-observability/30-01-SUMMARY.md.planning/phases/30-web-app-observability/30-02-PLAN.md.planning/phases/30-web-app-observability/30-02-SUMMARY.md.planning/phases/30-web-app-observability/30-03-PLAN.md.planning/phases/30-web-app-observability/30-03-SUMMARY.md.planning/phases/30-web-app-observability/30-04-PLAN.md.planning/phases/30-web-app-observability/30-04-SUMMARY.md.planning/phases/30-web-app-observability/30-CONTEXT.md.planning/phases/30-web-app-observability/30-DISCUSSION-LOG.md.planning/phases/30-web-app-observability/30-VERIFICATION.mdapps/web/package.jsonapps/web/src/App.cssapps/web/src/App.tsxapps/web/src/components/ErrorFallback.tsxapps/web/src/hooks/useAuth.tsapps/web/src/lib/faro.tsapps/web/src/main.tsxapps/web/src/vite-env.d.tsapps/web/vite.config.tspackage.json
- Add `email` to SENSITIVE_KEYS for broader PII coverage - Handle arrays in scrubObject to catch sensitive values in lists - Scrub item.meta in beforeSend, not just payload - Return redacted placeholder at depth limit instead of raw object - Scrub logger context before forwarding to Faro transport - Require GRAFANA_STACK_ID for Faro plugin enablement - Only generate hidden source maps when Faro upload is active - Add aria-hidden to decorative "// ERROR" text - Remove stale "(NEEDS DISCUSSION)" from Phase 30 roadmap entry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 4e42e59b270c
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/src/vite-env.d.ts (1)
2-6: MissingImportMetainterface for proper type augmentation.The
ImportMetaEnvinterface is declared but not connected toImportMeta. For the types to be recognized onimport.meta.env, you need to augment theImportMetainterface as well.♻️ Proposed fix
/// <reference types="vite/client" /> interface ImportMetaEnv { readonly VITE_FARO_URL?: string; readonly VITE_APP_VERSION?: string; } + +interface ImportMeta { + readonly env: ImportMetaEnv; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/vite-env.d.ts` around lines 2 - 6, The ImportMetaEnv type isn't connected to import.meta because the ImportMeta interface is missing; add an augmentation for ImportMeta so that import.meta.env is typed as ImportMetaEnv (i.e., declare the ImportMeta interface with a readonly env property of type ImportMetaEnv) so the VITE_* vars on import.meta.env are properly recognized; update the file that declares ImportMetaEnv to also declare ImportMeta with env: ImportMetaEnv.
🤖 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/src/lib/faro.ts`:
- Around line 38-63: scrubObject currently lets nested arrays and objects deeper
than 3 levels pass unmodified, so secrets inside arrays or 4+ level subtrees
remain unredacted; update scrubObject and its use of scrubValue so arrays are
recursively processed (call scrubValue/scrubObject on each array element) and
change the depth-handling behavior: when depth exceeds the 3-level limit, do not
return the original subtree—either map the subtree to scrubbed values or replace
it with a safe redaction marker—so no branch can bypass SENSITIVE_KEYS or
scrubValue checks; ensure scrubValue handles Uint8Array/binary-like values and
hex-like strings so those are redacted when encountered during the recursive
processing.
- Around line 11-20: The telemetry redaction is incomplete: update the privacy
gate used in beforeSend to fully scrub emails and other PII by (1) adding the
plain key 'email' to the SENSITIVE_KEYS set and (2) implementing a traversal in
the beforeSend handler that recursively inspects event objects (including
meta.user, nested objects, and arrays) and redacts any property whose key
matches SENSITIVE_KEYS (e.g., userEmail, email, meta.user.email) and any string
value matching an email regex pattern; ensure the traversal replaces sensitive
values with a constant placeholder and leaves non-matching data intact so that
all email-like strings anywhere in the event are removed before sending.
---
Nitpick comments:
In `@apps/web/src/vite-env.d.ts`:
- Around line 2-6: The ImportMetaEnv type isn't connected to import.meta because
the ImportMeta interface is missing; add an augmentation for ImportMeta so that
import.meta.env is typed as ImportMetaEnv (i.e., declare the ImportMeta
interface with a readonly env property of type ImportMetaEnv) so the VITE_* vars
on import.meta.env are properly recognized; update the file that declares
ImportMetaEnv to also declare ImportMeta with env: ImportMetaEnv.
🪄 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: 5278d5e7-9b20-4cf9-baa9-daffb7f30b08
⛔ Files ignored due to path filters (2)
packages/api-client/openapi.jsonis excluded by!packages/api-client/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.github/workflows/deploy-staging.yml.markdownlintignore.planning/STATE.md.planning/config.json.planning/phases/28-code-hygiene-logging/28-CONTEXT.md.planning/phases/28-code-hygiene-logging/28-DISCUSSION-LOG.md.planning/phases/29-infrastructure-hardening/29-VERIFICATION.md.planning/phases/30-web-app-observability/30-01-PLAN.md.planning/phases/30-web-app-observability/30-01-SUMMARY.md.planning/phases/30-web-app-observability/30-02-PLAN.md.planning/phases/30-web-app-observability/30-02-SUMMARY.md.planning/phases/30-web-app-observability/30-03-PLAN.md.planning/phases/30-web-app-observability/30-03-SUMMARY.md.planning/phases/30-web-app-observability/30-04-PLAN.md.planning/phases/30-web-app-observability/30-04-SUMMARY.md.planning/phases/30-web-app-observability/30-CONTEXT.md.planning/phases/30-web-app-observability/30-DISCUSSION-LOG.md.planning/phases/30-web-app-observability/30-VERIFICATION.mdapps/web/package.jsonapps/web/src/App.cssapps/web/src/App.tsxapps/web/src/components/ErrorFallback.tsxapps/web/src/hooks/useAuth.tsapps/web/src/lib/faro.tsapps/web/src/main.tsxapps/web/src/vite-env.d.tsapps/web/vite.config.tspackage.json
- Add URL_HEX_KEY_PATTERN to redact hex key material embedded in URLs (e.g., invite link hash fragments with ephemeral private keys) - Add @grafana/faro-web-tracing with ignoreNetworkEvents to prevent request/response bodies from reaching telemetry - Prevents encrypted payload leaks through fetch/XHR instrumentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 58e5f764436c
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #386 +/- ##
=======================================
Coverage 60.55% 60.55%
=======================================
Files 128 128
Lines 9601 9601
Branches 938 938
=======================================
Hits 5814 5814
Misses 3573 3573
Partials 214 214
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…option - Add @grafana/faro-web-tracing to package.json (was missing from prior commit) - Remove invalid captureConsoleDisabledLevels option Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: dab4f6a76ae1
Global regex lastIndex must be reset before .test() to avoid missing matches when scrubValue is called repeatedly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e4038f43b131
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 `@apps/web/src/lib/faro.ts`:
- Around line 160-167: The clearFaroUser function uses faro.api.setUser(null as
any) to clear identity; replace that workaround by calling faro.api.resetUser()
instead. In the clearFaroUser function (which gets the Faro instance via
getFaroInstance()), check for a truthy faro and then invoke faro.api.resetUser()
to clear the user identity using the SDK’s dedicated API.
🪄 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: b9074c0f-4eb1-4160-ad22-eb6edac70730
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.planning/ROADMAP.mdapps/web/package.jsonapps/web/src/components/ErrorFallback.tsxapps/web/src/lib/faro.tsapps/web/vite.config.ts
✅ Files skipped from review due to trivial changes (2)
- .planning/ROADMAP.md
- apps/web/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/ErrorFallback.tsx
- apps/web/vite.config.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 0dae3861ae40
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/faro.ts (1)
72-80: Nested arrays lose their array shape during scrubbing.When an array contains another array, the inner array is cast to
Record<string, unknown>and processed byscrubObject, which returns an object with numeric string keys instead of preserving the array structure.Example:
['a', ['b', 'c']]→['a', { '0': 'b', '1': 'c' }]Privacy is maintained (data is scrubbed), but telemetry shape differs from source. This is likely rare in Faro payloads, so flagging as optional.
♻️ Proposed fix to preserve array shape
if (Array.isArray(value)) { - result[key] = value.map((item) => { - const s = scrubValue(item); - if (s !== item) return s; - if (typeof item === 'object' && item !== null) { - return scrubObject(item as Record<string, unknown>, depth + 1); - } - return item; - }); + result[key] = scrubArray(value, depth + 1); } else if (typeof value === 'object' && value !== null) {Add a helper function:
function scrubArray(arr: unknown[], depth: number): unknown[] { if (depth > 3) return ['[DEPTH_LIMIT]']; return arr.map((item) => { const s = scrubValue(item); if (s !== item) return s; if (Array.isArray(item)) return scrubArray(item, depth + 1); if (typeof item === 'object' && item !== null) { return scrubObject(item as Record<string, unknown>, depth + 1); } return item; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/faro.ts` around lines 72 - 80, The current scrub path treats nested arrays as objects (losing array shape) because when Array.isArray(value) items that are arrays fall through to scrubObject; add a helper function scrubArray(arr: unknown[], depth: number) that mirrors scrubObject's depth-limit logic (e.g., returns ['[DEPTH_LIMIT]'] when depth > 3) and maps items by calling scrubValue, recursing into scrubArray for inner arrays and scrubObject for objects; then update the Array.isArray(value) branch to call scrubArray(value, depth + 1) instead of mapping that casts inner arrays to objects, keeping scrubValue and scrubObject calls intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/lib/faro.ts`:
- Around line 72-80: The current scrub path treats nested arrays as objects
(losing array shape) because when Array.isArray(value) items that are arrays
fall through to scrubObject; add a helper function scrubArray(arr: unknown[],
depth: number) that mirrors scrubObject's depth-limit logic (e.g., returns
['[DEPTH_LIMIT]'] when depth > 3) and maps items by calling scrubValue,
recursing into scrubArray for inner arrays and scrubObject for objects; then
update the Array.isArray(value) branch to call scrubArray(value, depth + 1)
instead of mapping that casts inner arrays to objects, keeping scrubValue and
scrubObject calls intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac498a38-fcf8-4b83-bf2b-967240690bfa
📒 Files selected for processing (1)
apps/web/src/lib/faro.ts
Extract scrubArray helper so nested arrays are recursed properly instead of being cast to Record<string, unknown> and losing their array structure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 61d51880f29d
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
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.
Summary
beforeSendgate that scrubs crypto keys, hex material, and email from all telemetryFaroErrorBoundarywith terminal-aesthetic fallback UI reassuring users their encrypted data is saferegisterFaroTransport)Configuration Required
VITE_FARO_URL,GRAFANA_FARO_API_KEY,GRAFANA_STACK_IDin GitHub staging environmentVITE_FARO_URLis absent (local dev)Test plan
pnpm --filter web build)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation