[AB-738] Implement FE/BE crash reports & metrix #1212
Conversation
WalkthroughAdds end-to-end observability: new frontend/backend registries and providers, Sentry adapters, metric primitives and safe wrappers, global UI error handling, build/runtime Sentry config, instrumentation points (metrics, traces, error captures) and runtime wiring for UI and ASGI server. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Frontend App
participant ObsRegFE as ObservabilityRegistry (FE)
participant SentryFE as SentryAdapter (FE)
participant SentrySDK as "@sentry/vue"
App->>ObsRegFE: initializeProvider(app, router)
ObsRegFE->>SentryFE: resolve & initialize()
alt DSN present & enabled
SentryFE->>SentrySDK: dynamic import & init(config)
SentrySDK-->>SentryFE: ready
SentryFE-->>ObsRegFE: initialized
else missing/disabled
SentryFE-->>ObsRegFE: no-op / logged
end
Note right of App: later calls use ObsRegFE.getInitializedProvider()
sequenceDiagram
autonumber
participant User as Client Action
participant Core as Core (WebSocket)
participant Metrics as frontendMetrics
participant ObsRegFE as ObservabilityRegistry
participant SentryFE as SentryAdapter
User->>Core: sendFrontendMessage(type, payload, cb)
Core->>Core: record start timestamps
Core->>Metrics: record/send WebSocketMessageDuration
Metrics->>ObsRegFE: getInitializedProvider()
ObsRegFE->>SentryFE: recordDistribution / incrementMetric
alt callback/error
Core->>Metrics: increment WebSocketMessageError
Metrics->>ObsRegFE: incrementMetric(websocket.message_error)
end
sequenceDiagram
autonumber
participant Server as ASGI startup
participant ObsRegBE as ObservabilityRegistry (BE)
participant SentryBE as SentryAdapter (BE)
participant FastAPI as FastAPI App
Server->>ObsRegBE: get_registry(app_path)
ObsRegBE->>SentryBE: register & initialize()
alt initialized & enabled
SentryBE->>FastAPI: instrument_fastapi_app(app)
Note right of FastAPI: per-request Sentry scope, tags, capture exceptions
else not initialized
Note right of Server: continue without instrumentation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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
🧹 Nitpick comments (17)
src/ui/vite.config.ts (1)
24-26: Consider a lower default for traces sample rate in production.The default
VITE_SENTRY_TRACES_SAMPLE_RATEof 1.0 (100%) could generate excessive trace data and impact performance in high-traffic scenarios. Consider defaulting to a lower value like 0.1 (10%).Apply this diff:
- VITE_SENTRY_TRACES_SAMPLE_RATE: JSON.stringify( - process.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "1.0", - ), + VITE_SENTRY_TRACES_SAMPLE_RATE: JSON.stringify( + process.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", + ),src/ui/.env (1)
4-4: Consider reducing trace sample rate for production.A
VITE_SENTRY_TRACES_SAMPLE_RATEof 1.0 (100%) in production can generate significant overhead and costs. Consider using a lower value such as 0.1 (10%) for production environments.Apply this diff:
-VITE_SENTRY_TRACES_SAMPLE_RATE=1.0 +VITE_SENTRY_TRACES_SAMPLE_RATE=0.1src/ui/src/main.ts (1)
15-16: Frontend observability wiring looks solid; consider guarding against duplicate global handlers and normalizing error typesThe observability provider init, page-load metric, and global error handlers are all wired in a safe, non-fatal way, which is good. Two small follow-ups you might consider:
- Because
load()can be retried frominitialise(), if it ever throws after the listeners are attached, you'll add multiplewindowlisteners. A simple one-time guard (e.g., a module-level flag) around theaddEventListenercalls would prevent duplicates.- For the
"error"handler you almost always end up witherrorType === error.name(e.g.,"Error","TypeError"), so the"window_error"fallback is rarely used. If you care about distinguishing sources, you might want to pass an explicit error type (e.g.,"window_error"/"unhandled_promise_rejection") in both handlers and rely less onerror.name.Also applies to: 64-92
src/ui/src/observability/index.ts (1)
1-15: Static import means missing Sentry deps will fail before thetryblockThe registry setup and guarded
register("sentry", new SentryAdapter())look fine. Note, though, that becauseSentryAdapteris imported statically at the top of the module, missing adapter dependencies (e.g.,@sentry/vue) will cause a module‑load failure before thetry/catchruns. If you truly want Sentry to be optional at runtime, you may want to move the adapter import behind a dynamic import or makeSentryAdapteritself handle missing external deps internally.src/ui/src/core/index.ts (2)
191-201: WebSocket keepalive latency metric is correct; consider guardingperformancefor non-browser contextsMeasuring latency around
"keepAlive"viaperformance.now()andtrackWebSocketLatencyis a good fit here. IfgenerateCore()is ever used in non‑browser or headless test environments, you might want to guard against a missingperformanceglobal (e.g.,if (typeof performance === "undefined")), but in standard browser-only usage this is fine.
734-846: Message duration and interaction metrics are well placed; minor simplifications possibleThe timing and metric recording in
sendFrontendMessageare wired correctly:
wrappedCallbacktracks interaction duration for non‑keepAlivemessages.websocket.message_durationandwebsocket.message_errorare sent only when a provider with the right capability is initialized, and metric failures are swallowed.Two small cleanups you could consider:
messageStartTimeandinteractionStartTimeare captured back‑to‑back; unless you intentionally want them to differ, a single timestamp reused for both would simplify things.- The repeated
getInitializedProvider()+ structural cast pattern is cropping up in several places; long‑term, a small helper aroundrecordDistribution/incrementMetricon top of the registry could reduce boilerplate.src/writer/observability/__init__.py (1)
16-32: Narrow the broadExceptioncatch and avoid repeated__all__mutationsCatching
Exceptionin_register_sentry_adapteravoids startup failures, but it also masks real programming errors insideSentryAdapter(not just missing imports). Similarly, callingget_registry()repeatedly can append"SentryAdapter"to__all__multiple times.You might tighten this up by:
- Catching
ImportErrorseparately for the optional dependency case, and logging other exceptions withexc_info=Trueso they’re visible.- Guarding
__all__updates and registration so they only happen once (e.g., by checking whether"sentry"is already registered or"SentryAdapter"is already in__all__).src/ui/src/observability/frontendMetrics.ts (3)
3-12: Consider modern Navigation Timing API for page load metrics
window.performance.timingis legacy and may be phased out in some environments. Consider preferringperformance.getEntriesByType("navigation")[0]when available and falling back toperformance.timingto future‑proof this helper.
13-31: Deduplicate metrics method typing via a shared interfaceEach helper redefines a local type for
recordDistribution/incrementMetric. This is correct but brittle if signatures change. Consider defining a sharedMetricsObservabilityProvider(e.g., extendingObservabilityProviderwith optionalincrementMetric?,recordDistribution?,setGauge?) and reusing it here instead of ad‑hoc casts.Also applies to: 45-54, 75-84, 103-112, 134-143
55-62: Revisiterror_messageas a metric tag to avoid high cardinality/PIIIncluding (even truncated)
error_messageas a tag can create very high cardinality series and may duplicate potentially sensitive content already sent in error events themselves. Consider limiting the metric toerror_type(plus maybe a coarse error code) and moving full messages into Sentry events only.src/ui/src/observability/sentryAdapter.ts (5)
114-116: AlignisEnabledwith DSN presence to avoid noisy failed initsCurrently
isEnabledreturns true unlessVITE_SENTRY_ENABLED === "false", so the registry will pick Sentry even when no DSN is configured, only to haveinitializelog and returnfalse. Consider treating “missing DSN” as effectively disabled so the registry can fall back cleanly:- isEnabled(): boolean { - return import.meta.env[SENTRY_ENABLED_ENV] !== "false"; - } + isEnabled(): boolean { + const enabledFlag = import.meta.env[SENTRY_ENABLED_ENV]; + const dsn = import.meta.env[SENTRY_DSN_ENV]; + if (enabledFlag === "false") return false; + // Only consider Sentry enabled when a DSN is configured + return !!dsn; + }This keeps behavior explicit while avoiding repeated failed initialization attempts.
Also applies to: 131-139
200-209: Harden sample‑rate parsing and clamp to a safe range
parseFloaton env values can yieldNaNor out‑of‑range numbers, which then get passed directly to Sentry. Consider normalizing:- const tracesSampleRate = parseFloat( - import.meta.env[SENTRY_TRACES_SAMPLE_RATE_ENV] || "1.0", - ); - const replaySampleRate = parseFloat( - import.meta.env[SENTRY_REPLAY_SAMPLE_RATE_ENV] || "0.1", - ); + const normalizeRate = (value: string, fallback: number): number => { + const parsed = parseFloat(value); + if (Number.isNaN(parsed)) return fallback; + return Math.min(Math.max(parsed, 0), 1); + }; + + const tracesSampleRate = normalizeRate( + import.meta.env[SENTRY_TRACES_SAMPLE_RATE_ENV] || "1.0", + 1.0, + ); + const replaySampleRate = normalizeRate( + import.meta.env[SENTRY_REPLAY_SAMPLE_RATE_ENV] || "0.1", + 0.1, + );This guards against misconfigured env values.
306-312: Guard access towindow/navigatorfor SSR or non‑browser environments
captureException,captureMessage, and_setInitialMetadataassumewindowandnavigatorexist. If this adapter is ever imported or exercised in SSR/tests without a DOM, these calls will throw. A small guard makes it safer:- const runtimeContext = { - url: window.location.href, - userAgent: navigator.userAgent, - }; + if (typeof window === "undefined" || typeof navigator === "undefined") { + // Non‑browser environment; skip browser‑specific context. + this.sentry.captureException(error instanceof Error ? error : new Error(String(error))); + return; + } + const runtimeContext = { + url: window.location.href, + userAgent: navigator.userAgent, + };and similarly gate
window.location,navigator.userAgent, andnavigator.languagein other methods.Also applies to: 355-367, 438-442
469-488: Clarify route metric ownership vs. external callers
_setupRouteTrackingalready incrementsfrontend_route_changes_totalon routerafterEach. Combined withtrackRouteChangeinfrontendMetrics.ts, there’s potential for duplicated metrics if both are wired. It would help to:
- Decide whether router‑level tracking is the sole producer of this metric, and
- Document (or enforce) that external callers should not emit the same metric, or use a distinct name.
This keeps route metrics consistent and avoids double counting.
559-567: Cache metrics instance after lazy lookup to avoid repeated discovery
_getMetricsInstancefalls back to_getMetrics(this.sentryModule)on every call but doesn’t cache the result. Minor, but you can avoid redundant lookups:- if (this.sentryModule) { - return this._getMetrics(this.sentryModule); - } - return null; + if (this.sentryModule) { + this.metrics = this.metrics ?? this._getMetrics(this.sentryModule); + return this.metrics; + } + return null;This keeps the lazy semantics while reusing the discovered metrics handle.
src/ui/src/observability/base.ts (2)
1-21: Consider adding optional metric APIs toObservabilityProviderCallers like
frontendMetricsandSentryAdapterrely onincrementMetric/recordDistribution/setGaugevia ad‑hoc casts and"in"checks. To make this contract explicit and reduce duplication, consider extending the interface (or adding a sub‑interface) with optional metric methods:export interface ObservabilityProvider { // existing methods... incrementMetric?( name: string, options?: { tags?: Record<string, string>; unit?: string; value?: number }, ): void; recordDistribution?( name: string, value: number, options?: { tags?: Record<string, string>; unit?: string }, ): void; setGauge?( name: string, value: number, options?: { tags?: Record<string, string>; unit?: string }, ): void; }This keeps the base type aligned with how providers are actually used.
87-129: Review comment is valid and recommended for implementationThe suggestion to normalize
initializeProviderto an always-async API is sound. Verification shows:
- Single call site at
src/ui/src/main.ts:65already usesawait, making it future-proof- Provider interface at
src/ui/src/observability/base.ts:2currently allowsboolean | Promise<boolean>, but the only implementation (SentryAdapter) returnsPromise<boolean>- Minimal impact: No test files affected, only one real usage
- Code simplification: Eliminates the
result instanceof Promisebranching logic and reduces complexityTo implement this refactoring, update:
ObservabilityRegistry.initializeProvider→async initializeProvider(...): Promise<boolean>ObservabilityProviderinterface →initialize(...): Promise<boolean>or mark as async- Replace the if/else branching with
const result = await provider.initialize(app, router);- The call site (main.ts) is already compatible
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpoetry.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
pyproject.toml(1 hunks)src/ui/.env(1 hunks)src/ui/package.json(1 hunks)src/ui/src/composables/useLogger.ts(1 hunks)src/ui/src/composables/useWriterTracking.ts(2 hunks)src/ui/src/core/index.ts(4 hunks)src/ui/src/core/navigation.ts(3 hunks)src/ui/src/main.ts(4 hunks)src/ui/src/observability/base.ts(1 hunks)src/ui/src/observability/frontendMetrics.ts(1 hunks)src/ui/src/observability/index.ts(1 hunks)src/ui/src/observability/sentryAdapter.ts(1 hunks)src/ui/vite.config.ts(1 hunks)src/writer/observability/__init__.py(1 hunks)src/writer/observability/base.py(1 hunks)src/writer/observability/sentry_adapter.py(1 hunks)src/writer/serve.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-22T11:08:20.053Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1100
File: src/ui/src/composables/useFilesEncoder/useFilesEncoder.ts:14-22
Timestamp: 2025-08-22T11:08:20.053Z
Learning: The writer-framework project does not require SSR (Server-Side Rendering) support, so using `window` object is acceptable in the codebase.
Applied to files:
src/ui/vite.config.ts
🧬 Code graph analysis (10)
src/ui/src/observability/index.ts (1)
src/ui/src/observability/base.ts (1)
ObservabilityRegistry(23-161)
src/ui/src/main.ts (2)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)src/ui/src/observability/frontendMetrics.ts (2)
trackPageLoadTime(3-38)trackError(40-68)
src/ui/src/composables/useLogger.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
src/writer/observability/base.py (1)
src/ui/src/observability/base.ts (3)
ObservabilityProvider(1-21)ObservabilityRegistry(23-161)register(27-34)
src/writer/serve.py (2)
src/writer/observability/__init__.py (1)
get_registry(30-32)src/writer/observability/base.py (2)
initialize_provider(51-84)instrument_app(86-94)
src/ui/src/core/navigation.ts (1)
src/ui/src/observability/frontendMetrics.ts (1)
trackRouteChange(98-124)
src/ui/src/observability/frontendMetrics.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
src/writer/observability/sentry_adapter.py (1)
src/writer/observability/base.py (4)
ObservabilityProvider(8-27)is_enabled(25-27)initialize(12-14)instrument_fastapi_app(17-19)
src/ui/src/composables/useWriterTracking.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
src/ui/src/core/index.ts (3)
src/ui/src/observability/frontendMetrics.ts (2)
trackWebSocketLatency(70-96)trackInteractionDuration(126-159)src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)src/ui/src/writerTypes.ts (1)
Component(25-44)
🪛 dotenv-linter (4.0.0)
src/ui/.env
[warning] 5-5: [UnorderedKey] The VITE_SENTRY_REPLAY_SAMPLE_RATE key should go before the VITE_SENTRY_TRACES_SAMPLE_RATE key
(UnorderedKey)
🪛 Ruff (0.14.5)
src/writer/serve.py
172-172: Do not catch blind exception: Exception
(BLE001)
src/writer/observability/sentry_adapter.py
69-69: Unused function argument: hint
(ARG001)
110-110: Consider moving this statement to an else block
(TRY300)
161-161: Consider moving this statement to an else block
(TRY300)
src/writer/observability/__init__.py
26-26: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.13)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
- GitHub Check: tests (chromium)
- GitHub Check: tests (firefox)
- GitHub Check: tests (webkit)
🔇 Additional comments (11)
src/ui/src/core/navigation.ts (1)
1-1: LGTM! Route change tracking is well integrated.The implementation correctly tracks route changes at all appropriate points (explicit hash changes and browser hashchange events), with proper error handling and sensible defaults.
Also applies to: 92-110
src/ui/src/composables/useWriterTracking.ts (1)
189-212: LGTM! Observability metrics integration is well implemented.The metric tracking is appropriately added alongside existing event tracking, with proper error handling and structured metric naming (
user_action.{eventName}).src/writer/serve.py (1)
167-173: LGTM! Observability initialization is appropriately defensive.The optional observability setup correctly ensures that failures don't break the application startup. The broad exception handling on line 172 is intentional and appropriate here, as observability is a non-critical feature that should never prevent the application from starting.
src/ui/src/composables/useLogger.ts (1)
3-3: LGTM! Logger observability integration is clean and non-breaking.The integration maintains full backward compatibility while adding observability capture for warnings and errors. The implementation correctly handles different argument types and includes useful metadata.
Also applies to: 8-42
pyproject.toml (1)
57-57: Consider updating the Sentry SDK to a more recent version within the ^2.x range.The current constraint
^2.15.0is free from the known security vulnerabilities in the GitHub security advisories (those affect versions < 2.8.0 or < 1.14.0). However, version 2.15.0 is significantly outdated—the latest available is 2.46.0. Updating to allow newer patch and minor versions will ensure access to the latest bug fixes, performance improvements, and security patches. You can update the constraint to^2.15.0or simply allow updates to^2.46.0depending on your stability requirements.src/ui/package.json (1)
51-51: No issues found—@sentry/vue@10.25.0 is compatible with Vue 3.5.0 and no security advisories detected.The package version is compatible with Vue 3.5.0, and the latest available version is 10.26.0. The caret constraint (^10.25.0) permits automatic updates to the latest minor version, so no action is required.
src/ui/src/main.ts (2)
164-171: Good use of observability registry for initialization failuresCapturing the core initialization failure via
observabilityRegistry.captureExceptionwith structured context (source,component,stage) is a clean way to surface fatal startup issues without coupling directly to Sentry.
102-108: Remove the review comment—this change is correct and fixes a type mismatchThe verification shows:
WriterApiUserProfile.idis typenumberUserCollaborationPing.userIdexpects typenumberThe new code correctly passes
writerProfile.id(anumber) directly toupdateOutgoingPing. The old code with.toString()would have sent a string, which violates the type contract. This change aligns the code with the correct types—it's a bug fix, not a breaking change.src/ui/src/core/index.ts (1)
848-878: Component add/delete metrics look correct and safeEmitting
component.deletedandcomponent.addedmetrics with acomponent_typetag right where components are mutated is a good hook point. Thecomponentexistence check before deletion and the defensivetry/catcharound the metric call avoid surprises if no observability provider is initialized or if the provider throws.Also applies to: 880-907
src/writer/observability/sentry_adapter.py (1)
119-178: FastAPI middleware integration is sound; be mindful of when middleware can be addedThe custom
SentryMiddlewarecorrectly:
- Uses
push_scope()per request.- Tags request/response metadata and captures exceptions via
sentry_sdk.capture_exception.- Handles missing FastAPI/Starlette/imports and already‑started apps without breaking startup.
The control flow and exception handling look good. Once you address the PII handling in the request context (comment above), this will be a solid integration.
src/writer/observability/base.py (1)
8-99: Backend observability registry implementation looks solidThe provider abstraction and registry lifecycle (env selection, first‑enabled fallback, initialization, and instrumentation) are straightforward and mirror the frontend design. Error paths are logged and safely handled; no correctness issues stand out.
| @@ -0,0 +1,5 @@ | |||
| VITE_SENTRY_DSN=https://6c13c3260114c5b1ceb3b6ae58d0cdb3@o1026471.ingest.us.sentry.io/4510398008393728 | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify .env file is not committed to version control.
The Sentry DSN is exposed in this .env file. While the DSN is a public key, environment files should typically not be committed to version control as they can contain sensitive configuration. Ensure this file is properly listed in .gitignore, or consider using environment variables at runtime instead.
🏁 Script executed:
#!/bin/bash
# Check if .env is in .gitignore
grep -r "^\.env$" .gitignore
# Check if this .env file is tracked by git
git ls-files src/ui/.envLength of output: 11
Let me verify the .gitignore status to ensure we have complete information:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check .gitignore content directly
cat .gitignore | grep -E "\.env"
Length of output: 42
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for .gitignore files anywhere in repo
fd -name .gitignore
# Also check if there's a root-level gitignore (alternative names)
ls -la | grep -i git
Length of output: 407
---
🏁 Script executed:
```shell
#!/bin/bash
# Check .gitignore content for .env entries
cat .gitignore
Length of output: 425
Remove src/ui/.env from version control and add it to .gitignore.
The .env file containing the Sentry DSN is currently tracked by git and is not listed in .gitignore. This violates security best practices and exposes environment configuration in version control history.
Add .env to .gitignore and remove the file from git tracking using git rm --cached src/ui/.env. Consider using a .env.example template instead for documenting required environment variables.
🤖 Prompt for AI Agents
In src/ui/.env at line 1, the committed .env contains a Sentry DSN and must be
removed from version control; add src/ui/.env (or just .env) to .gitignore, run
git rm --cached src/ui/.env to stop tracking the file while keeping it locally,
commit the .gitignore change and the removal, and optionally add a
src/ui/.env.example with placeholder variable names (e.g. VITE_SENTRY_DSN=) to
document required environment variables.
There was a problem hiding this comment.
You need to use config.js. You can take a look at this PR which does that: https://github.com/WriterInternal/fs.agent-flow/pull/89 .
IMO, it needs to be a new hook like
function useWriterConfiguration(core: Core) {
onMounted(() => {
if (wf.isWriterCloudApp.value) fetchConfig()
});
return config;
}|
|
||
| self._agent_id = os.getenv("WRITER_APP_ID") | ||
| self._org_id = os.getenv("WRITER_ORG_ID") | ||
| environment = os.getenv(SENTRY_ENVIRONMENT_ENV, "production") |
There was a problem hiding this comment.
We don't have this environment variable, so it'll be prodduction for Test and dev environment.
There was a problem hiding this comment.
Maybe we can use WRITER_BASE_URL?
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
HackerOne Code Security Review
🟢 Scan Complete: 23 Issue(s)
Here's how the code changes were interpreted and info about the tools used for scanning.
📖 Summary of Changes
The changes introduce comprehensive observability and error tracking across the application using Sentry. New modules and configurations were added to both frontend (Vue/TypeScript) and backend (Python) components to enable error monitoring, performance tracking, and metrics collection. The implementation includes global error handling, metric tracking for various events, and Sentry SDK integration with detailed context and logging capabilities.| File | Summary |
|---|---|
| pyproject.toml | Added sentry-sdk dependency with FastAPI extras to the poetry dependencies section, which appears to be the only change in the configuration file. |
| src/ui/.env | A new .env file was added with Sentry configuration settings, including DSN, environment, and sampling rates for error tracking and performance monitoring in a production context. |
| src/ui/package.json | A new dependency "@sentry/vue" with version "^10.25.0" was added to the dependencies section of the package.json file. |
| src/ui/src/composables/useBlueprintNodeTools.ts | No changes were made to this file. The old and new versions are identical, with an extra newline added at the end of the new version. |
| src/ui/src/composables/useGlobalErrorHandling.ts | A new file is introduced that implements global error handling for web applications, with functions to set up and tear down error event listeners, extract and track errors, and handle both window errors and unhandled promise rejections. |
| src/ui/src/composables/useLogger.ts | The updated file adds an import for observabilityRegistry and enhances warn and error methods to capture messages and exceptions using an observability provider, with additional metadata and error handling. |
| src/ui/src/composables/useWriterTracking.ts | The file now includes an import for frontend metrics and adds a call to incrementMetricSafely within the track function, which increments a metric with the event name and includes additional metadata about the event type. |
| src/ui/src/core/index.ts | Added observability metrics to WebSocket message handling and component management functions, including tracking WebSocket latency, message duration, interaction duration, and component addition/deletion events. |
| src/ui/src/core/navigation.ts | Added import for route change tracking metric, and introduced a new trackRouteChange function that is called in changePageInHash and changeRouteVarsInHash methods. Also added a hashchange event listener to track route changes when window is available. |
| src/ui/src/main.ts | Added observability-related imports and functions, including global error handling setup, observability registry initialization, page load time tracking, and exception capturing during core initialization. |
| src/ui/src/observability/base.ts | The file introduces a new TypeScript module defining an ObservabilityProvider interface and an ObservabilityRegistry class for managing and initializing observability providers with methods for registering, initializing, and capturing exceptions and messages. |
| src/ui/src/observability/frontendMetrics.ts | A new TypeScript file was added defining enums for metric names, types, and units, along with utility functions for tracking various frontend metrics like page load time, errors, WebSocket latency, route changes, and interaction durations. |
| src/ui/src/observability/index.ts | A new file was created that imports and exports ObservabilityRegistry and SentryAdapter, initializes an observabilityRegistry, and attempts to register a Sentry adapter with error handling via a debug console log. |
| src/ui/vite.config.ts | Added Sentry configuration environment variables to the define section, with default values for DSN, traces sample rate, and replay sample rate. |
| src/writer/app_runner.py | Added Sentry initialization in the app process's _main method with error handling and logging. The new code attempts to initialize the Sentry provider and logs success or failure messages, providing observability for potential initialization issues. |
| src/writer/blueprints.py | Added Sentry error tracking for blueprint execution errors, with detailed context and tags for better error monitoring and debugging. Introduced a logger and modified exception handling to capture and report errors more comprehensively. |
| src/writer/core.py | The changes in the file primarily involve adding Sentry error tracking in the _handle_global_event method of the EventHandler class. This includes importing and using Sentry SDK to capture and log exceptions with additional context when an error occurs during global event handling. |
| src/writer/observability/init.py | A new observability integration module was created, introducing an ObservabilityRegistry, a Sentry adapter registration function, and a method to retrieve the registry with optional app path configuration. |
| src/writer/observability/base.py | The file introduces a new observability framework with an abstract base class ObservabilityProvider and a registry class ObservabilityRegistry, providing mechanisms for registering, initializing, and instrumenting observability providers for applications. |
| src/writer/observability/sentry_adapter.py | The file is a new implementation of a Sentry observability provider adapter for a Python application. It includes methods for initializing Sentry, checking if Sentry is enabled, instrumenting FastAPI applications, and configuring Sentry SDK with metadata, logging integration, and error handling. |
| src/writer/serve.py | Added observability initialization with Sentry error tracking in the get_asgi_app function, and modified the blueprint job creation endpoint to capture exceptions with Sentry. Also added Sentry context and tags for better error tracking and debugging. |
ℹ️ Issues Detected
NOTE: These may not require action!
Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.
How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:
- Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
- Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.
| File & Line | Issue |
|---|---|
src/ui/.env Line 1 |
This file contains a Sentry DSN which includes a project identifier and authentication token. While this appears to be intended for client-side error reporting, exposing DSNs in version control is generally not recommended as they should be managed through environment variables or deployment configurations. |
src/writer/core.py Line 1900 |
The code introduces a new error handling mechanism that uses Sentry SDK to capture exceptions. However, it directly uses user-controlled data from ev.type and ev.handler in the Sentry context without proper validation or sanitization. This could potentially lead to injection of malicious data into Sentry logs or dashboards if an attacker can control these values. Additionally, the code catches all exceptions when trying to use Sentry, which could hide important security-related errors. |
src/writer/observability/sentry_adapter.py Line 164 |
The SentryMiddleware implementation at lines 143-174 includes sensitive request data in the Sentry context. While there's an attempt to filter sensitive headers in the before_send function (lines 76-84), the middleware itself adds query parameters (line 164) to the Sentry context without filtering potentially sensitive information that might be present in those parameters. |
src/ui/src/observability/sentryAdapter.ts Line 195 |
The code hardcodes 'production' as the environment value in the Sentry initialization config (line 195), regardless of the actual environment the application is running in. This could lead to development or staging errors being reported as production issues, potentially causing confusion in error tracking and metrics. |
src/writer/app_runner.py Line 504 |
The code initializes Sentry (an error tracking service) without proper validation of user input. The app_path parameter is passed directly to the observability registry without sanitization, which could potentially lead to path traversal if the app_path is manipulated. An attacker could potentially use this to access sensitive files or execute code outside the intended directory. |
src/writer/blueprints.py Line 789 |
The exception handling code added in lines 779-790 and 797-818 catches all exceptions when trying to report to Sentry, but silently suppresses any errors that might occur during this process with a bare except Exception: pass. This could hide important configuration or integration issues with Sentry. Consider logging these exceptions instead of silently ignoring them. |
src/writer/serve.py Line 610 |
The code imports and uses sentry_sdk without proper validation of user input. In the error handling for blueprint API execution (lines 603-621), the code captures exceptions with Sentry and includes user-provided blueprint_id and branch_id as tags. If these values contain malicious content, they could be used to inject data into monitoring systems. The code should sanitize these values before using them with Sentry. |
src/writer/blueprints.py Line 789 |
Try, Except, Pass detected. |
src/writer/serve.py Line 620 |
Try, Except, Pass detected. |
src/writer/blueprints.py Line 817 |
Try, Except, Pass detected. |
src/writer/observability/sentry_adapter.py Line 26 |
self._org_id: Optional[str] is a built-in function and should be avoided as a variable name. Using language-provided function names as variable names can complicate future development since any attempt to call that specific function within this scope may result in unintended behaviors. |
src/writer/observability/base.py Line 34 |
self._providers: Dict[str, ObservabilityProvider] is a built-in function and should be avoided as a variable name. Using language-provided function names as variable names can complicate future development since any attempt to call that specific function within this scope may result in unintended behaviors. |
src/ui/.env Line 3 |
Committing a .env file to version control is against best practice and can often cause secrets to be committed to a repository even if unintentional. It is acceptable to include a .env.example that does not contain any sensitive data and can be easily copied to a .env as a starting template for necessary environment variables. Consider adding .env to your .gitignore as it should be only added locally to each environment where the code is run. |
src/ui/.env Line 2 |
Committing a .env file to version control is against best practice and can often cause secrets to be committed to a repository even if unintentional. It is acceptable to include a .env.example that does not contain any sensitive data and can be easily copied to a .env as a starting template for necessary environment variables. Consider adding .env to your .gitignore as it should be only added locally to each environment where the code is run. |
src/writer/observability/sentry_adapter.py Line 25 |
self._agent_id: Optional[str] is a built-in function and should be avoided as a variable name. Using language-provided function names as variable names can complicate future development since any attempt to call that specific function within this scope may result in unintended behaviors. |
src/ui/.env Line 1 |
Committing a .env file to version control is against best practice and can often cause secrets to be committed to a repository even if unintentional. It is acceptable to include a .env.example that does not contain any sensitive data and can be easily copied to a .env as a starting template for necessary environment variables. Consider adding .env to your .gitignore as it should be only added locally to each environment where the code is run. |
src/ui/src/observability/sentryAdapter.ts Line 641 |
Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string. |
src/ui/src/observability/sentryAdapter.ts Line 617 |
Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string. |
src/ui/src/observability/sentryAdapter.ts Line 661 |
Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string. |
src/ui/src/observability/base.ts Line 108 |
Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string. |
src/ui/.env Line 4 |
Committing a .env file to version control is against best practice and can often cause secrets to be committed to a repository even if unintentional. It is acceptable to include a .env.example that does not contain any sensitive data and can be easily copied to a .env as a starting template for necessary environment variables. Consider adding .env to your .gitignore as it should be only added locally to each environment where the code is run. |
src/ui/.env Line 5 |
Committing a .env file to version control is against best practice and can often cause secrets to be committed to a repository even if unintentional. It is acceptable to include a .env.example that does not contain any sensitive data and can be easily copied to a .env as a starting template for necessary environment variables. Consider adding .env to your .gitignore as it should be only added locally to each environment where the code is run. |
src/ui/src/observability/base.ts Line 129 |
Detected string concatenation with a non-literal variable in a util.format / console.log function. If an attacker injects a format specifier in the string, it will forge the log message. Try to use constant values for the format string. |
🧰 Analysis tools
- [ ✅ ] HackerOne AI Code Analysis
- [ ✅ ] HackerOne AI Code Validation
- [ ✅ ] semgrep
- [ ✅ ] bandit
⏱️ Latest scan covered changes up to commit 4912adf (latest)
There was a problem hiding this comment.
This is quite a large MR, and will require a few passes to review in full. I'll keep you updated as the review progresses.
Reviewed with ❤️ by PullRequest
|
|
||
| self._agent_id = os.getenv("WRITER_APP_ID") | ||
| self._org_id = os.getenv("WRITER_ORG_ID") | ||
| environment = os.getenv(SENTRY_ENVIRONMENT_ENV, "production") |
There was a problem hiding this comment.
Maybe we can use WRITER_BASE_URL?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/writer/observability/sentry_adapter.py (2)
149-154: PII leakage concern with URL and query parameters remains unaddressed.This was flagged in a previous review: full URLs and query parameters may contain tokens, API keys, or user identifiers that would be sent to Sentry unfiltered. The
before_sendhook sanitizes headers but doesn't touchevent.requestor query strings.
61-65: Environment detection may misreport test/dev as production.As noted in a previous review, without
SENTRY_ENVIRONMENTset, all environments default to"production". Consider deriving environment from other signals (e.g.,WRITER_BASE_URLor a dedicated config) to avoid polluting production error data with test traffic.src/ui/src/observability/frontendMetrics.ts (1)
98-115: Deduplication mitigates but doesn't resolve the double-counting issue flagged in past reviews.While the
lastTrackedRouteguard prevents consecutive identical routes from being double-counted, the prior review identified that bothnavigation.ts(lines 79, 89) andSentryAdapter._setupRouteTrackingcall into route-change tracking for the same navigation events. If these code paths pass different tag values (e.g.,pageKeyvs. routername/path), deduplication won't catch them. The recommended fix was to choose a single source of truth—either remove manualtrackRouteChangecalls where hash navigation occurs, or disable the router hook inSentryAdapter—so only one path emits the metric.Based on past review comments, consolidate to a single tracking source rather than relying solely on deduplication.
🧹 Nitpick comments (6)
src/ui/src/composables/useGlobalErrorHandling.ts (1)
17-24: Inconsistent error type tagging for window errors.Line 20 uses
error.name || "window_error"as the error type, but for genericErrorobjects,error.nameis always"Error", so"window_error"is never used as fallback. This differs from the consistent"unhandled_promise_rejection"used inhandleUnhandledRejection. Consider using"window_error"consistently:function handleError(event: ErrorEvent): void { try { const error = extractError(event.error || event.message); - trackError(error, error.name || "window_error"); + trackError(error, "window_error"); } catch (trackingError) { console.error("Failed to track error:", trackingError); } }src/ui/src/core/index.ts (1)
748-760: Redundant variable assignment.
messageStartTimeandinteractionStartTimeare assigned the same value on consecutive lines. SinceinteractionStartTimeis only used in the callback andmessageStartTimeis used for the send duration, you could simplify by using a single timestamp:- const messageStartTime = performance.now(); - const interactionStartTime = performance.now(); + const messageStartTime = performance.now(); const wrappedCallback = callback ? (response: { ok: boolean; payload?: unknown }) => { if (response?.ok && type !== "keepAlive") { - const duration = - performance.now() - interactionStartTime; + const duration = performance.now() - messageStartTime; trackInteractionDuration(type, duration); } callback(response); } : undefined;src/writer/observability/__init__.py (2)
19-32: Repeated adapter registration on eachget_registry()call.
_register_sentry_adapter()is called every timeget_registry()is invoked, which will attempt to re-register the adapter. WhileObservabilityRegistry.register()handles duplicates with a warning, this is inefficient and noisy. Consider adding a guard:+_sentry_registered = False + + def _register_sentry_adapter(app_path: Optional[str] = None): + global _sentry_registered + if _sentry_registered: + return try: from writer.observability.sentry_adapter import SentryAdapter sentry_adapter = SentryAdapter(app_path=app_path) if sentry_adapter.is_enabled(): _observability_registry.register("sentry", sentry_adapter) __all__.append("SentryAdapter") + _sentry_registered = True except Exception as e: logger.debug(f"Sentry adapter not available: {e}")
24-25: Mutating__all__at runtime may cause duplicates and hinders static analysis.If
_register_sentry_adapteris called multiple times,"SentryAdapter"could be appended repeatedly. Also, static analysis tools won't recognize this dynamic export. Consider checking before appending:if sentry_adapter.is_enabled(): _observability_registry.register("sentry", sentry_adapter) - __all__.append("SentryAdapter") + if "SentryAdapter" not in __all__: + __all__.append("SentryAdapter")src/writer/observability/sentry_adapter.py (1)
69-77: Rename unusedhintparameter to satisfy linter.The
hintparameter is required by Sentry's callback signature but unused here.- def before_send(event, hint): + def before_send(event, _hint): """Filter sensitive data before sending to Sentry."""src/ui/src/observability/sentryAdapter.ts (1)
187-192: Sample rates not clamped to valid range.Unlike the backend adapter (which clamps
traces_sample_rateto[0.0, 1.0]), this code uses the parsed values directly. Invalid values (negative or >1.0) could cause unexpected Sentry behavior.const tracesSampleRate = parseFloat( import.meta.env[SENTRY_TRACES_SAMPLE_RATE_ENV] || "1.0", ); + const clampedTracesSampleRate = Math.max(0, Math.min(1, tracesSampleRate)); const replaySampleRate = parseFloat( import.meta.env[SENTRY_REPLAY_SAMPLE_RATE_ENV] || "0.1", ); + const clampedReplaySampleRate = Math.max(0, Math.min(1, replaySampleRate)); const initConfig: SentryInitConfig = { dsn, environment, - tracesSampleRate, + tracesSampleRate: clampedTracesSampleRate, replay: { - sampleRate: replaySampleRate, + sampleRate: clampedReplaySampleRate, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/ui/src/composables/useBlueprintNodeTools.ts(1 hunks)src/ui/src/composables/useGlobalErrorHandling.ts(1 hunks)src/ui/src/composables/useLogger.ts(1 hunks)src/ui/src/composables/useWriterTracking.ts(2 hunks)src/ui/src/core/index.ts(4 hunks)src/ui/src/main.ts(5 hunks)src/ui/src/observability/base.ts(1 hunks)src/ui/src/observability/frontendMetrics.ts(1 hunks)src/ui/src/observability/sentryAdapter.ts(1 hunks)src/writer/observability/__init__.py(1 hunks)src/writer/observability/sentry_adapter.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/ui/src/composables/useBlueprintNodeTools.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/ui/src/composables/useLogger.ts
- src/ui/src/main.ts
- src/ui/src/composables/useWriterTracking.ts
- src/ui/src/observability/base.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/ui/src/composables/useGlobalErrorHandling.ts (1)
src/ui/src/observability/frontendMetrics.ts (1)
trackError(75-83)
src/ui/src/core/index.ts (1)
src/ui/src/observability/frontendMetrics.ts (4)
trackWebSocketLatency(85-96)trackInteractionDuration(117-131)recordDistributionSafely(170-193)incrementMetricSafely(141-159)
src/ui/src/observability/frontendMetrics.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
src/writer/observability/__init__.py (2)
src/writer/observability/base.py (4)
ObservabilityProvider(8-27)ObservabilityRegistry(30-98)is_enabled(25-27)register(37-41)src/writer/observability/sentry_adapter.py (1)
is_enabled(28-30)
src/ui/src/observability/sentryAdapter.ts (3)
src/ui/src/observability/index.ts (2)
SentryAdapter(6-6)ObservabilityProvider(4-4)src/ui/src/observability/base.ts (1)
ObservabilityProvider(2-22)src/ui/src/observability/frontendMetrics.ts (1)
trackRouteChange(100-115)
src/writer/observability/sentry_adapter.py (1)
src/ui/src/observability/base.ts (1)
ObservabilityProvider(2-22)
🪛 Ruff (0.14.5)
src/writer/observability/__init__.py
26-26: Do not catch blind exception: Exception
(BLE001)
src/writer/observability/sentry_adapter.py
69-69: Unused function argument: hint
(ARG001)
110-110: Consider moving this statement to an else block
(TRY300)
159-159: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: tests (webkit)
- GitHub Check: build (3.13)
- GitHub Check: build (3.9)
- GitHub Check: build (3.12)
- GitHub Check: tests (chromium)
- GitHub Check: tests (firefox)
🔇 Additional comments (12)
src/ui/src/composables/useGlobalErrorHandling.ts (1)
35-52: LGTM!Clean implementation with proper guards for SSR environments and double-initialization prevention. The pattern of storing handler references for proper cleanup is correct.
src/ui/src/core/index.ts (2)
789-816: LGTM!Good separation of concerns: message send duration (local serialization + send time) vs. interaction duration (round-trip callback). Error metrics are properly tagged.
819-852: LGTM!Component lifecycle metrics are properly instrumented. Good pattern of capturing component type before deletion and consistent use of
incrementMetricSafelywith logger.src/writer/observability/sentry_adapter.py (1)
119-165: LGTM on middleware structure.The middleware correctly uses
push_scopefor request-scoped tags and captures exceptions with proper re-raising. The error handling for middleware addition (lines 169-173) appropriately handles the "app already started" case.src/ui/src/observability/sentryAdapter.ts (2)
108-271: LGTM on initialization flow.Robust initialization with proper fallbacks for BrowserTracing integration, error handling for dynamic imports, and correct Vue/router integration. The async initialization pattern allows for graceful degradation when Sentry SDK is unavailable.
569-634: LGTM on metric methods.Consistent pattern across
incrementMetric,recordDistribution, andsetGaugewith proper guards and error handling. The_buildAttributeshelper correctly enriches metrics with app metadata.src/ui/src/observability/frontendMetrics.ts (6)
3-46: LGTM: Well-structured type definitions.The enums, interfaces, and provider types provide good type safety and clear contracts for metric operations.
75-83: LGTM: Good defensive programming.The function correctly limits error message length to prevent high cardinality and provides sensible fallbacks for missing error information.
85-96: LGTM: Appropriate metric type for latency.Using a distribution metric for latency is correct and allows for percentile calculations.
117-131: LGTM: Clean interaction tracking.The function correctly uses a distribution metric for duration tracking with appropriate units and tags.
133-159: LGTM: Addresses past feedback on wrapping metric calls.This utility function eliminates the need for inline type checks and repetitive error handling when incrementing metrics, directly addressing the prior review comment about wrapping function calls.
Based on past review comments, this consolidates the common pattern requested.
161-193: LGTM: Consistent safe wrapper pattern.This function mirrors the
incrementMetricSafelypattern, providing consistent error handling and type checking for distribution metrics.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/ui/src/observability/frontendMetrics.ts (3)
26-35: Consider tightening theunitoption type to reduce mistakes
RecordDistributionOptions.unitandIncrementMetricOptions.unitare plainstringwhile you already exposeMetricUnitas the canonical set. If you don’t expect non-standard units, making theseunit?: MetricUnit(orMetricUnit | stringif you need extensibility) would catch typos at compile time without changing runtime behavior.
49-84: Navigation Timing v2 usage looks good; consider a lightweight fallback pathNice move to
performance.getEntriesByType("navigation")and usingPerformanceNavigationTiming. In environments wherewindow.performanceexists butgetEntriesByType("navigation")isn’t implemented or returns no entries,loadTimestaysnulland you silently skip emitting the metric.If you care about broader browser coverage, you could:
- Guard
performance.getEntriesByTypewith atypeof === "function"check, and- As a fallback, use the legacy
window.performance.timingcalculation whennavigationEntriesis empty or the fields are missing.Example sketch:
- const navigationEntries = performance.getEntriesByType( - "navigation", - ) as PerformanceNavigationTiming[]; + const navigationEntries = + typeof performance.getEntriesByType === "function" + ? (performance.getEntriesByType("navigation") as PerformanceNavigationTiming[]) + : []; if (navigationEntries.length > 0) { const entry = navigationEntries[0]; if (entry.loadEventEnd && entry.startTime) { loadTime = entry.loadEventEnd - entry.startTime; } else if (entry.duration) { loadTime = entry.duration; } } + + if (loadTime === null && window.performance.timing) { + const timing = window.performance.timing; + if (timing.loadEventEnd && timing.navigationStart) { + loadTime = timing.loadEventEnd - timing.navigationStart; + } + }This keeps the modern path while preserving metrics where only the old API is available.
105-122: Route de-dupe is good; ensure caller passes a normalized route identifier
lastTrackedRoutede-dupes immediate repeats, which should prevent simple double-counting from the same source. However, theroutetag is whatever string the caller passes; if that’s a full path with IDs/query params, you can still get high-cardinality metrics.To keep the metric usable, make sure call sites pass a normalized identifier (e.g., router name or templated path) and consider documenting that expectation in this function’s JSDoc or in the call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/ui/src/composables/useBlueprintNodeTools.ts(1 hunks)src/ui/src/composables/useWriterTracking.ts(2 hunks)src/ui/src/core/index.ts(4 hunks)src/ui/src/observability/frontendMetrics.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/ui/src/composables/useBlueprintNodeTools.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/ui/src/composables/useWriterTracking.ts
- src/ui/src/core/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/ui/src/observability/frontendMetrics.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
🔇 Additional comments (2)
src/ui/src/observability/frontendMetrics.ts (2)
140-155: Safe increment wrapper behavior is solidThe
incrementMetricSafelyhelper is a good pattern: it no-ops cleanly when no provider is initialized and isolates provider failures behind atry/catch, preventing observability from breaking the UI. The"incrementMetric" in providerguard keeps you resilient to partial provider implementations.
157-174: Distribution wrapper mirrors increment wrapper correctly
recordDistributionSafelymirrors the increment helper’s structure and error isolation, giving a consistent and safe surface for all metric producers. No changes needed here.
| export function trackError(error: Error, errorType?: string): void { | ||
| incrementMetricSafely(MetricName.FrontendErrorsTotal, { | ||
| tags: { | ||
| error_type: errorType || error.name || "unknown", | ||
| error_message: error.message?.substring(0, 100) || "unknown", | ||
| }, | ||
| unit: MetricUnit.None, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Avoid using error_message as a metric tag to prevent cardinality and cost blowups
Tagging metrics with (truncated) error.message will likely create extremely high-cardinality time series, which is problematic for most metric backends (performance, storage, alert noise) and hard to aggregate meaningfully. You already have FrontendErrorsTotal and a useful error_type dimension; the raw message is better sent to logs/traces/Sentry rather than encoded into metric tags.
I’d recommend dropping error_message from tags (or replacing it with a low-cardinality code/bucket), e.g.:
export function trackError(error: Error, errorType?: string): void {
incrementMetricSafely(MetricName.FrontendErrorsTotal, {
tags: {
- error_type: errorType || error.name || "unknown",
- error_message: error.message?.substring(0, 100) || "unknown",
+ error_type: errorType || error.name || "unknown",
},
unit: MetricUnit.None,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function trackError(error: Error, errorType?: string): void { | |
| incrementMetricSafely(MetricName.FrontendErrorsTotal, { | |
| tags: { | |
| error_type: errorType || error.name || "unknown", | |
| error_message: error.message?.substring(0, 100) || "unknown", | |
| }, | |
| unit: MetricUnit.None, | |
| }); | |
| } | |
| export function trackError(error: Error, errorType?: string): void { | |
| incrementMetricSafely(MetricName.FrontendErrorsTotal, { | |
| tags: { | |
| error_type: errorType || error.name || "unknown", | |
| }, | |
| unit: MetricUnit.None, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/ui/src/observability/frontendMetrics.ts around lines 86 to 94, the metric
tags include error_message which can create very high-cardinality time series;
remove the error_message tag from the MetricName.FrontendErrorsTotal call and
only keep a low-cardinality tag such as error_type (errorType || error.name ||
"unknown"); if you need the full message, send error.message to
logs/traces/Sentry instead or map it to a small set of buckets/error codes
before tagging.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/ui/src/observability/sentryAdapter.ts (1)
9-86: Consider importing types directly from @sentry/vue.The manual interface definitions increase maintenance overhead and may drift from the actual SDK types over time. Since
@sentry/vueis already imported as a type (line 5), consider importing the relevant types directly from the SDK.Note: This concern was previously raised in past review comments.
🧹 Nitpick comments (1)
src/ui/src/observability/sentryAdapter.ts (1)
387-424: Cache_getAppMetadata()result to avoid duplicate calls.The
_getAppMetadata()method is called twice (lines 397 and 413), performing the same complex property access both times. Consider caching the result in a local variable.Apply this diff:
private _setInitialMetadata(SentryModule: SentryModule): void { if (!this.initialized || !SentryModule) { return; } try { + const appMetadata = this._getAppMetadata(); + if (SentryModule.setTag) { SentryModule.setTag("platform", "frontend"); SentryModule.setTag("framework", "writer-framework"); - const appMetadata = this._getAppMetadata(); if (appMetadata.agent_id) { SentryModule.setTag("agent_id", appMetadata.agent_id); } if (appMetadata.organization_id) { SentryModule.setTag( "organization_id", appMetadata.organization_id, ); } if (appMetadata.mode) { SentryModule.setTag("mode", appMetadata.mode); } } if (SentryModule.setContext) { - const appMetadata = this._getAppMetadata(); SentryModule.setContext("application", appMetadata); SentryModule.setContext("runtime", { name: "browser", user_agent: navigator.userAgent, language: navigator.language, }); } } catch (e) { console.warn("Failed to set initial Sentry metadata:", e); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ui/src/observability/sentryAdapter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/ui/src/observability/sentryAdapter.ts (3)
src/ui/src/observability/base.ts (1)
ObservabilityProvider(2-22)src/ui/src/core/navigation.ts (1)
getParsedHash(25-37)src/ui/src/observability/frontendMetrics.ts (1)
trackRouteChange(107-122)
🔇 Additional comments (1)
src/ui/src/observability/sentryAdapter.ts (1)
268-313: LGTM! Well-structured error capture with proper guards.The method correctly handles both
Errorobjects and string messages, includes appropriate runtime context, and has proper error handling. The type assertions are reasonable given Sentry's dynamic API.
| async initialize(app?: unknown, router?: unknown): Promise<boolean> { | ||
| if (this.initialized) { | ||
| return true; | ||
| } | ||
|
|
||
| if (router) { | ||
| this.router = router; | ||
| } | ||
|
|
||
| try { | ||
| const dsn = import.meta.env[SENTRY_DSN_ENV]; | ||
| if (!dsn) { | ||
| console.info( | ||
| "Sentry DSN not provided, skipping Sentry initialization", | ||
| ); | ||
| return false; | ||
| } | ||
|
|
||
| let SentryModule: SentryModule; | ||
| try { | ||
| const sentryImport = await import( | ||
| /* @vite-ignore */ "@sentry/vue" | ||
| ); | ||
| SentryModule = sentryImport as unknown as SentryModule; | ||
| } catch (importError) { | ||
| console.warn( | ||
| "Failed to import @sentry/vue package. Make sure it's installed: npm install @sentry/vue", | ||
| importError, | ||
| ); | ||
| return false; | ||
| } | ||
|
|
||
| const { | ||
| init, | ||
| captureException, | ||
| captureMessage, | ||
| setUser, | ||
| setContext, | ||
| } = SentryModule; | ||
|
|
||
| const integrations: unknown[] = []; | ||
|
|
||
| if (SentryModule.vueIntegration) { | ||
| integrations.push( | ||
| SentryModule.vueIntegration({ | ||
| app: app || undefined, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| let browserTracingIntegration: BrowserTracingIntegration | null = | ||
| null; | ||
| if (SentryModule.browserTracingIntegration) { | ||
| browserTracingIntegration = | ||
| SentryModule.browserTracingIntegration; | ||
| } else if (SentryModule.BrowserTracing) { | ||
| browserTracingIntegration = SentryModule.BrowserTracing; | ||
| } else { | ||
| try { | ||
| const browserModule = (await import( | ||
| /* @vite-ignore */ "@sentry/browser" | ||
| )) as { | ||
| browserTracingIntegration?: BrowserTracingIntegration; | ||
| }; | ||
| if (browserModule.browserTracingIntegration) { | ||
| browserTracingIntegration = | ||
| browserModule.browserTracingIntegration; | ||
| } | ||
| } catch { | ||
| console.warn( | ||
| "BrowserTracing not available. Performance monitoring will be limited.", | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const apiBaseUrl = | ||
| import.meta.env.VITE_WRITER_BASE_URL ?? window.location.origin; | ||
|
|
||
| const tracesSampleRate = parseFloat( | ||
| import.meta.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", | ||
| ); | ||
| const replaySampleRate = parseFloat( | ||
| import.meta.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1", | ||
| ); | ||
|
|
||
| const initConfig: SentryInitConfig = { | ||
| dsn, | ||
| environment: apiBaseUrl, | ||
| tracesSampleRate, | ||
| replay: { | ||
| sampleRate: replaySampleRate, | ||
| }, | ||
| enableMetrics: true, | ||
| }; | ||
|
|
||
| if (app) { | ||
| initConfig.app = app; | ||
| this.app = app; | ||
| } | ||
|
|
||
| if (browserTracingIntegration) { | ||
| const tracingOptions: BrowserTracingOptions = { | ||
| tracingOrigins: ["localhost", /^\//], | ||
| }; | ||
|
|
||
| if (this.router) { | ||
| tracingOptions.router = this.router; | ||
| tracingOptions.routeLabel = "path"; | ||
| } | ||
|
|
||
| if ( | ||
| typeof browserTracingIntegration === "function" && | ||
| !browserTracingIntegration.prototype | ||
| ) { | ||
| const integration = | ||
| browserTracingIntegration as BrowserTracingFunction; | ||
| integrations.push(integration(tracingOptions)); | ||
| } else { | ||
| const IntegrationClass = | ||
| browserTracingIntegration as BrowserTracingClass; | ||
| integrations.push(new IntegrationClass(tracingOptions)); | ||
| } | ||
| } | ||
|
|
||
| if (integrations.length > 0) { | ||
| initConfig.integrations = integrations; | ||
| } | ||
|
|
||
| init(initConfig); | ||
|
|
||
| this.sentry = { | ||
| captureException, | ||
| captureMessage, | ||
| setUser, | ||
| setContext, | ||
| }; | ||
|
|
||
| this.sentryModule = SentryModule; | ||
| this.metrics = this._getMetrics(SentryModule); | ||
|
|
||
| if (this.metrics) { | ||
| console.debug("Sentry metrics API initialized"); | ||
| } else { | ||
| console.warn( | ||
| "Sentry metrics API not available (requires SDK 10.25.0+)", | ||
| ); | ||
| } | ||
|
|
||
| this.initialized = true; | ||
|
|
||
| this._setInitialMetadata(SentryModule); | ||
| this._setupRouteTracking(); | ||
|
|
||
| console.info(`Sentry initialized (environment: ${apiBaseUrl})`); | ||
| return true; | ||
| } catch (error) { | ||
| console.warn( | ||
| "Failed to load or initialize @sentry/vue package:", | ||
| error, | ||
| ); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Break down the initialize method into smaller functions.
The initialize method is over 160 lines and handles multiple distinct responsibilities: guard checks, dynamic imports, integration setup, configuration building, SDK initialization, metadata setup, and route tracking. This makes it difficult to understand, test, and maintain.
Consider extracting helper methods:
_loadSentryModule(): Handle dynamic import of @sentry/vue_setupIntegrations(SentryModule, app): Build integrations array_createInitConfig(dsn, integrations): Build init configuration_setupBrowserTracing(SentryModule): Extract lines 154-226
This would make the initialize method a high-level orchestrator that's easier to follow.
🤖 Prompt for AI Agents
In src/ui/src/observability/sentryAdapter.ts around lines 104-266, the
initialize method is too large and mixes multiple responsibilities; refactor by
extracting small private helpers: implement _loadSentryModule() to perform the
dynamic import and return the typed module or null,
_setupIntegrations(SentryModule, app?, router?) to assemble and return the
integrations array (including vue integration and browser tracing creation),
_setupBrowserTracing(SentryModule, app?, router?) to encapsulate the browser
tracing selection and instantiation logic (formerly lines ~154-226), and
_createInitConfig(dsn, integrations, app?) to build and return the
SentryInitConfig object; update initialize to call these helpers in sequence,
handle their return values/early exits, set
this.sentry/this.metrics/this.initialized and call
_setInitialMetadata/_setupRouteTracking as before so initialize becomes a
high-level orchestrator.
| const tracesSampleRate = parseFloat( | ||
| import.meta.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", | ||
| ); | ||
| const replaySampleRate = parseFloat( | ||
| import.meta.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1", | ||
| ); |
There was a problem hiding this comment.
Validate parsed sample rates to prevent NaN.
parseFloat on environment variables without validation can produce NaN if the values are invalid, which would be passed to Sentry configuration. Consider adding validation or using a helper that ensures valid numeric values.
Apply this diff to add validation:
const tracesSampleRate = parseFloat(
import.meta.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1",
);
+if (isNaN(tracesSampleRate)) {
+ console.warn("Invalid VITE_SENTRY_TRACES_SAMPLE_RATE, defaulting to 0.1");
+ tracesSampleRate = 0.1;
+}
const replaySampleRate = parseFloat(
import.meta.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1",
);
+if (isNaN(replaySampleRate)) {
+ console.warn("Invalid VITE_SENTRY_REPLAY_SAMPLE_RATE, defaulting to 0.1");
+ replaySampleRate = 0.1;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tracesSampleRate = parseFloat( | |
| import.meta.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", | |
| ); | |
| const replaySampleRate = parseFloat( | |
| import.meta.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1", | |
| ); | |
| const tracesSampleRate = (() => { | |
| const value = parseFloat( | |
| import.meta.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", | |
| ); | |
| if (isNaN(value)) { | |
| console.warn("Invalid VITE_SENTRY_TRACES_SAMPLE_RATE, defaulting to 0.1"); | |
| return 0.1; | |
| } | |
| return value; | |
| })(); | |
| const replaySampleRate = (() => { | |
| const value = parseFloat( | |
| import.meta.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1", | |
| ); | |
| if (isNaN(value)) { | |
| console.warn("Invalid VITE_SENTRY_REPLAY_SAMPLE_RATE, defaulting to 0.1"); | |
| return 0.1; | |
| } | |
| return value; | |
| })(); |
🤖 Prompt for AI Agents
In src/ui/src/observability/sentryAdapter.ts around lines 182 to 187, the code
parses sample-rate env vars with parseFloat but doesn’t validate the result;
update the parsing to validate and sanitize the values so NaN or out-of-range
numbers aren’t passed to Sentry: parse the env string, check Number.isFinite or
!Number.isNaN, default to a safe fallback (e.g., 0.1) when invalid, and clamp
the final value to the 0..1 range before using it for tracesSampleRate and
replaySampleRate.
| const initConfig: SentryInitConfig = { | ||
| dsn, | ||
| environment: apiBaseUrl, | ||
| tracesSampleRate, | ||
| replay: { | ||
| sampleRate: replaySampleRate, | ||
| }, | ||
| enableMetrics: true, | ||
| }; |
There was a problem hiding this comment.
Use semantic environment name instead of API base URL.
Setting environment to apiBaseUrl (e.g., "https://api.example.com") makes it difficult to filter and group errors in Sentry by deployment environment. Sentry's environment field should use semantic values like "production", "staging", or "development".
Consider using a dedicated environment variable:
const initConfig: SentryInitConfig = {
dsn,
- environment: apiBaseUrl,
+ environment: import.meta.env.VITE_SENTRY_ENVIRONMENT || "production",
tracesSampleRate,
replay: {
sampleRate: replaySampleRate,
},
enableMetrics: true,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const initConfig: SentryInitConfig = { | |
| dsn, | |
| environment: apiBaseUrl, | |
| tracesSampleRate, | |
| replay: { | |
| sampleRate: replaySampleRate, | |
| }, | |
| enableMetrics: true, | |
| }; | |
| const initConfig: SentryInitConfig = { | |
| dsn, | |
| environment: import.meta.env.VITE_SENTRY_ENVIRONMENT || "production", | |
| tracesSampleRate, | |
| replay: { | |
| sampleRate: replaySampleRate, | |
| }, | |
| enableMetrics: true, | |
| }; |
🤖 Prompt for AI Agents
In src/ui/src/observability/sentryAdapter.ts around lines 189 to 197, the Sentry
init is setting environment to apiBaseUrl (a URL) which is not semantic; change
it to use a semantic environment value (e.g. SENTRY_ENV or NODE_ENV mapped to
"production" | "staging" | "development") instead of the API URL. Read a
dedicated env var (SENTRY_ENV) with a safe fallback (e.g. map NODE_ENV ->
semantic value), validate it is one of the expected strings, and pass that into
the initConfig.environment field so Sentry receives a deployment environment
like "production" rather than an API URL.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/ui/src/observability/frontendMetrics.ts (1)
86-93: Removeerror_messagetag to prevent high-cardinality metric explosion.This was flagged in a previous review but appears unaddressed. The
error_messagetag (even truncated to 100 chars) creates extremely high-cardinality time series that degrade metric backend performance and are difficult to aggregate. The error details are better captured viacaptureExceptionbelow.export function trackError(error: Error, errorType?: string): void { incrementMetricSafely(MetricName.FrontendErrorsTotal, { tags: { error_type: errorType || error.name || "unknown", - error_message: error.message?.substring(0, 100) || "unknown", }, unit: MetricUnit.None, });
🧹 Nitpick comments (7)
src/ui/vite.config.ts (1)
17-23: Consider lowering default trace sample rate for production.The default
VITE_SENTRY_TRACES_SAMPLE_RATEof"1.0"means 100% of transactions will be traced, which can be expensive in production (both in terms of Sentry quota and performance overhead). A more typical production default is 0.1-0.2 (10-20%).The replay sample rate default of 0.1 is reasonable.
VITE_SENTRY_DSN: JSON.stringify(process.env.VITE_SENTRY_DSN || ""), VITE_SENTRY_TRACES_SAMPLE_RATE: JSON.stringify( - process.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "1.0", + process.env.VITE_SENTRY_TRACES_SAMPLE_RATE || "0.1", ), VITE_SENTRY_REPLAY_SAMPLE_RATE: JSON.stringify( process.env.VITE_SENTRY_REPLAY_SAMPLE_RATE || "0.1", ),src/writer/observability/sentry_adapter.py (1)
70-72: Consider lowering default trace sample rate for production.Similar to the frontend, the default
SENTRY_TRACES_SAMPLE_RATEof"1.0"means 100% sampling, which can be costly. Consider using a lower default like 0.1.environment = os.getenv(SENTRY_ENVIRONMENT_ENV, "production") - traces_sample_rate = float(os.getenv(SENTRY_TRACES_SAMPLE_RATE_ENV, "1.0")) + traces_sample_rate = float(os.getenv(SENTRY_TRACES_SAMPLE_RATE_ENV, "0.1")) traces_sample_rate = max(0.0, min(1.0, traces_sample_rate))src/ui/src/observability/frontendMetrics.ts (1)
119-134: Consider normalizing route tags to prevent cardinality issues.If
routecontains dynamic path segments (e.g.,/users/123), each unique value creates a new time series. Consider normalizing to route templates (e.g.,/users/:id) or using route names instead of full paths.src/writer/app_runner.py (1)
502-508: Log message assumes Sentry but could be any provider.The
initialize_provider()method initializes whichever provider is enabled (determined byOBSERVABILITY_PROVIDERenv var or first enabled provider). The log messages mentioning "Sentry" are misleading if a different provider is configured.try: from writer.observability import get_registry observability_registry = get_registry(app_path=self.app_path) if observability_registry.initialize_provider(): - self.logger.info("Sentry initialized in app process") + provider = observability_registry.get_initialized_provider() + provider_name = provider.get_name() if provider else "unknown" + self.logger.info(f"Observability provider '{provider_name}' initialized in app process") except Exception as e: - self.logger.warning(f"Failed to initialize Sentry in app process: {e}") + self.logger.warning(f"Failed to initialize observability provider in app process: {e}")src/writer/blueprints.py (2)
779-791: Consider importing sentry_sdk at module level.Importing
sentry_sdkinside the exception handler adds overhead on every error. Consider importing at the top of the module with a fallback pattern:+try: + import sentry_sdk + HAS_SENTRY = True +except ImportError: + HAS_SENTRY = False + logger = logging.getLogger(__name__)Then simplify the handler:
- try: - import sentry_sdk + if HAS_SENTRY: + try: - with sentry_sdk.push_scope() as scope: - scope.set_tag("source", "blueprint_execution") - scope.set_tag("error_type", "BlueprintExecutionError") - scope.set_context("blueprint", { - "run_id": self.run_id, - "title": self.status_logger.title, - }) - sentry_sdk.capture_exception(e) - except Exception: - pass + with sentry_sdk.push_scope() as scope: + scope.set_tag("source", "blueprint_execution") + scope.set_tag("error_type", "BlueprintExecutionError") + scope.set_context("blueprint", { + "run_id": self.run_id, + "title": self.status_logger.title, + }) + sentry_sdk.capture_exception(e) + except Exception: + logger.debug("Failed to report exception to Sentry", exc_info=True)This avoids repeated imports and adds debug logging for troubleshooting.
797-819: Consider extracting Sentry reporting to a helper function.This error reporting block is very similar to the one at lines 779-791, creating maintenance burden. Consider extracting a helper:
def _report_to_sentry(exception: Exception, run_id: str, title: str, **extra_tags): """Report exception to Sentry if available.""" if not HAS_SENTRY: return try: with sentry_sdk.push_scope() as scope: scope.set_tag("source", "blueprint_execution") scope.set_tag("error_type", type(exception).__name__) for key, value in extra_tags.items(): scope.set_tag(key, value) scope.set_context("blueprint", { "run_id": run_id, "title": title, }) sentry_sdk.capture_exception(exception) except Exception: logger.debug("Failed to report exception to Sentry", exc_info=True)Then use it in both handlers:
except BlueprintExecutionError as e: _report_to_sentry(e, self.run_id, self.status_logger.title) # ... existing error handling except BaseException as e: _report_to_sentry(e, self.run_id, self.status_logger.title, platform="backend", component="backend", layer="server") # ... existing error handlingsrc/writer/serve.py (1)
603-622: Apply the same optimization suggested for blueprints.py.This Sentry reporting block has the same efficiency issue as in
src/writer/blueprints.py(lines 779-791, 797-819). For consistency across the codebase, consider:
- Importing
sentry_sdkat module level with aHAS_SENTRYflag- Using the same helper function pattern if you implement it in
blueprints.pyThis ensures consistent error reporting behavior across blueprint execution contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/ui/src/observability/frontendMetrics.ts(1 hunks)src/ui/src/observability/sentryAdapter.ts(1 hunks)src/ui/vite.config.ts(1 hunks)src/writer/app_runner.py(1 hunks)src/writer/blueprints.py(2 hunks)src/writer/core.py(1 hunks)src/writer/observability/sentry_adapter.py(1 hunks)src/writer/serve.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/src/observability/sentryAdapter.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-22T11:08:20.053Z
Learnt from: VladKrasny
Repo: writer/writer-framework PR: 1100
File: src/ui/src/composables/useFilesEncoder/useFilesEncoder.ts:14-22
Timestamp: 2025-08-22T11:08:20.053Z
Learning: The writer-framework project does not require SSR (Server-Side Rendering) support, so using `window` object is acceptable in the codebase.
Applied to files:
src/ui/vite.config.ts
📚 Learning: 2025-06-16T15:34:16.165Z
Learnt from: ramedina86
Repo: writer/writer-framework PR: 935
File: src/writer/blueprints.py:210-212
Timestamp: 2025-06-16T15:34:16.165Z
Learning: In src/writer/blueprints.py, execution environments passed to run_branch and run_blueprint methods are intentionally mutated during execution. The environments need to be created beforehand and should not be deep-copied, as the mutation is part of the intended workflow design.
Applied to files:
src/writer/blueprints.py
🧬 Code graph analysis (5)
src/ui/src/observability/frontendMetrics.ts (1)
src/ui/src/observability/index.ts (1)
observabilityRegistry(8-8)
src/writer/observability/sentry_adapter.py (3)
src/ui/src/observability/base.ts (1)
ObservabilityProvider(2-22)src/writer/observability/base.py (4)
ObservabilityProvider(8-27)is_enabled(25-27)initialize(12-14)instrument_fastapi_app(17-19)src/ui/src/observability/sentryAdapter.ts (2)
SENTRY_DSN_ENV(7-7)initialize(104-270)
src/writer/serve.py (2)
src/writer/observability/__init__.py (1)
get_registry(30-32)src/writer/observability/base.py (2)
initialize_provider(51-84)instrument_app(86-94)
src/writer/app_runner.py (2)
src/writer/observability/__init__.py (1)
get_registry(30-32)src/writer/observability/base.py (1)
initialize_provider(51-84)
src/writer/blueprints.py (2)
src/writer/journal.py (1)
save(67-72)src/writer/keyvalue_storage.py (1)
save(49-55)
🪛 Ruff (0.14.5)
src/writer/observability/sentry_adapter.py
76-76: Unused function argument: hint
(ARG001)
124-124: Consider moving this statement to an else block
(TRY300)
170-170: Consider moving this statement to an else block
(TRY300)
src/writer/serve.py
172-172: Do not catch blind exception: Exception
(BLE001)
620-621: try-except-pass detected, consider logging the exception
(S110)
620-620: Do not catch blind exception: Exception
(BLE001)
src/writer/app_runner.py
507-507: Do not catch blind exception: Exception
(BLE001)
src/writer/blueprints.py
789-790: try-except-pass detected, consider logging the exception
(S110)
789-789: Do not catch blind exception: Exception
(BLE001)
795-795: Use raise without specifying exception name
Remove exception name
(TRY201)
817-818: try-except-pass detected, consider logging the exception
(S110)
817-817: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (2)
src/writer/blueprints.py (1)
20-20: LGTM!Module-level logger initialization follows Python best practices and supports the observability enhancements.
src/writer/serve.py (1)
167-173: LGTM!The observability initialization is properly defensive—failures are logged but don't prevent the app from starting. The broad exception catch is appropriate here for an optional feature.
| try: | ||
| import sentry_sdk | ||
| with sentry_sdk.push_scope() as scope: | ||
| scope.set_tag("platform", "backend") | ||
| scope.set_tag("component", "backend") | ||
| scope.set_tag("layer", "server") | ||
| scope.set_tag("source", "event_handler") | ||
| scope.set_tag("event_type", ev.type) | ||
| scope.set_context("event", { | ||
| "type": ev.type, | ||
| "handler": ev.handler, | ||
| "is_safe": ev.isSafe, | ||
| }) | ||
| scope.set_context("component", { | ||
| "type": "backend", | ||
| "platform": "backend", | ||
| "layer": "server", | ||
| "source": "event_handler", | ||
| }) | ||
| event_id = sentry_sdk.capture_exception(e) | ||
| if event_id: | ||
| logging.info(f"Error sent to Sentry (event_id: {event_id}, event_type: {ev.type})") | ||
| except ImportError: | ||
| logging.warning("Sentry SDK not available - error will not be sent to Sentry") | ||
| except Exception as sentry_error: | ||
| logging.error(f"Failed to send error to Sentry: {sentry_error}", exc_info=True) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Direct sentry_sdk import bypasses the observability abstraction layer.
This code directly imports and uses sentry_sdk, bypassing the ObservabilityRegistry and SentryAdapter abstraction established elsewhere in this PR. This creates several issues:
- If
SENTRY_ENABLED=false, this still attempts to use Sentry - The tag/context setup duplicates what
SentryAdapter.initialize()already configures globally - Inconsistent with
app_runner.pywhich uses the registry pattern
Consider using the registry's initialized provider instead:
except BaseException as e:
try:
- import sentry_sdk
- with sentry_sdk.push_scope() as scope:
- scope.set_tag("platform", "backend")
- scope.set_tag("component", "backend")
- scope.set_tag("layer", "server")
- scope.set_tag("source", "event_handler")
- scope.set_tag("event_type", ev.type)
- scope.set_context("event", {
- "type": ev.type,
- "handler": ev.handler,
- "is_safe": ev.isSafe,
- })
- scope.set_context("component", {
- "type": "backend",
- "platform": "backend",
- "layer": "server",
- "source": "event_handler",
- })
- event_id = sentry_sdk.capture_exception(e)
- if event_id:
- logging.info(f"Error sent to Sentry (event_id: {event_id}, event_type: {ev.type})")
- except ImportError:
- logging.warning("Sentry SDK not available - error will not be sent to Sentry")
- except Exception as sentry_error:
- logging.error(f"Failed to send error to Sentry: {sentry_error}", exc_info=True)
+ from writer.observability import get_registry
+ registry = get_registry()
+ provider = registry.get_initialized_provider()
+ if provider:
+ import sentry_sdk
+ with sentry_sdk.push_scope() as scope:
+ scope.set_tag("source", "event_handler")
+ scope.set_tag("event_type", ev.type)
+ scope.set_context("event", {
+ "type": ev.type,
+ "handler": ev.handler,
+ "is_safe": ev.isSafe,
+ })
+ sentry_sdk.capture_exception(e)
+ except Exception as obs_error:
+ logging.debug(f"Could not report error to observability provider: {obs_error}")🤖 Prompt for AI Agents
In src/writer/core.py around lines 1892-1917, replace the direct import/use of
sentry_sdk with the observability abstraction: obtain the initialized Sentry
provider from ObservabilityRegistry (or the SentryAdapter) and check that SENTRY
is enabled/that a provider instance exists before calling it; use the provider's
capture_exception API (or its scoped capture helper) to record the exception and
attach event metadata (event_type, handler, is_safe) via the provider's
supported parameters (e.g., extra/context) rather than re-setting global
tags/contexts, and preserve the ImportError/exception handling by logging a
warning if no provider is available and logging errors if sending fails.
| with sentry_sdk.configure_scope() as scope: | ||
| scope.set_tag("platform", "backend") | ||
| scope.set_tag("component", "backend") | ||
| scope.set_tag("layer", "server") | ||
| scope.set_tag("framework", "writer-framework") | ||
| if metadata["agent_id"]: | ||
| scope.set_tag("agent_id", metadata["agent_id"]) | ||
| if metadata["organization_id"]: | ||
| scope.set_tag("organization_id", metadata["organization_id"]) | ||
| if metadata.get("app_path"): | ||
| scope.set_tag("app_path", metadata["app_path"]) | ||
| scope.set_context("application", { | ||
| "path": metadata["app_path"], | ||
| "name": metadata.get("app_name"), | ||
| }) | ||
| scope.set_context("runtime", { | ||
| "name": "python", | ||
| "version": f"{sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}", | ||
| }) | ||
| scope.set_context("component", { | ||
| "type": "backend", | ||
| "platform": "backend", | ||
| "layer": "server", | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify configure_scope is not deprecated in your Sentry SDK version.
The configure_scope() context manager has been deprecated in Sentry SDK 2.x in favor of sentry_sdk.get_current_scope() or scope isolation patterns. Verify your SDK version and consider updating if you're on 2.x or later.
🌐 Web query:
Is sentry_sdk configure_scope deprecated in Python SDK 2.0?
💡 Result:
Short answer: Yes — sentry_sdk.configure_scope is deprecated in sentry-python 2.0 and marked for removal. Use the new scope APIs (get_current_scope / get_isolation_scope, new_scope, isolation_scope, or the Scope methods) instead. [1][2]
Sources:
- Migration guide (1.x → 2.x): explains configure_scope/push_scope deprecation and replacements. [1]
- sentry-python API docs: configure_scope marked "Deprecated since version 2.0.0". [2]
Update to non-deprecated Sentry scope APIs.
The configure_scope() context manager is deprecated since version 2.0.0 in sentry-python and marked for removal. Replace with get_current_scope(), get_isolation_scope(), new_scope, isolation_scope, or the Scope methods instead (lines 97–120).
🤖 Prompt for AI Agents
In src/writer/observability/sentry_adapter.py around lines 97–120, the code uses
the deprecated sentry_sdk.configure_scope() context manager; replace it with a
non-deprecated API such as sentry_sdk.push_scope() (to create an isolated
temporary scope) or
sentry_sdk.get_current_scope()/sentry_sdk.get_isolation_scope()/sentry_sdk.new_scope()
depending on desired isolation. Specifically, swap the configure_scope() usage
for sentry_sdk.push_scope() as the context manager (or obtain a scope via
get_current_scope() if you intend to mutate the global scope), then apply the
same scope.set_tag and scope.set_context calls to that scope so behavior remains
identical without using the deprecated API.
|
Graham C has submitted feedback.
Reviewed with ❤️ by PullRequest |
| @@ -0,0 +1,5 @@ | |||
| VITE_SENTRY_DSN=https://6c13c3260114c5b1ceb3b6ae58d0cdb3@o1026471.ingest.us.sentry.io/4510398008393728 | |||
There was a problem hiding this comment.
The Sentry DSN (Data Source Name) is hardcoded and committed to version control. This credential should be treated as a secret. Anyone with repository access can view this DSN, and it remains in git history even if removed in future commits. While Sentry DSNs are designed for client-side use (write-only), exposing them allows malicious actors to send false error reports to your project, potentially causing rate limit issues and polluting your error tracking data.
Committing environment-specific configuration files violates the principle of separating configuration from code. This DSN appears to be for production, which means developers might accidentally send development errors to production Sentry.
Remediation:
Add .env to .gitignore and create .env.example with placeholder values:
VITE_SENTRY_DSN=your_sentry_dsn_here
VITE_SENTRY_ENABLED=true
Configure actual values through your deployment platform's environment variables (Vercel, Netlify, AWS, etc.). Remove .env from git history using tools like BFG Repo-Cleaner. Consider rotating the exposed DSN in Sentry settings.
- https://cheatsheetseries.owasp.org/cheatsheets/Secrets_Management_Cheat_Sheet.html
- https://docs.sentry.io/product/accounts/quotas/
- https://owasp.org/www-community/vulnerabilities/Sensitive_Data_Exposure
🔺 Vulnerability (Error)
| return error; | ||
| } | ||
| if (typeof error === "string") { | ||
| return new Error(error); |
There was a problem hiding this comment.
The extractError function converts error strings to Error objects without validating or limiting message length. According to OWASP input validation guidelines, all external input should have maximum length constraints to prevent resource exhaustion. Excessively long error messages can cause memory issues, exceed API payload limits when sent to Sentry, and consume unnecessary storage in logs.
Error messages might include stack traces, large JSON payloads, or extensive debugging information. Without length limits, this creates a DoS vector where triggered errors with arbitrarily large messages could exhaust resources.
Remediation:
Implement maximum length validation:
const MAX_ERROR_LENGTH = 1500;
function extractError(error: unknown): Error {
if (error instanceof Error) {
if (error.message.length > MAX_ERROR_LENGTH) {
error.message = error.message.substring(0, MAX_ERROR_LENGTH) + '... (truncated)';
}
return error;
}
let msg = typeof error === "string" ? error : String(error || "Unknown error");
if (msg.length > MAX_ERROR_LENGTH) {
msg = msg.substring(0, MAX_ERROR_LENGTH) + '... (truncated)';
}
return new Error(msg);
}- https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html
- https://owasp.org/www-community/attacks/Denial_of_Service
- https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
🔸 Vulnerability (Warning)
| : new Error(String(args[0])); | ||
| provider.captureException(error, { | ||
| source: "logger", | ||
| component: "useLogger", |
There was a problem hiding this comment.
The error method passes all additional arguments to the observability provider as context without sanitization. Developers often include user objects, request payloads, or application state when logging errors. If any of this context contains passwords, API keys, tokens, PII, or other sensitive data, it will be sent to Sentry and accessible to all team members.
This violates data protection regulations and creates a security liability. Once sensitive data enters Sentry, it may be retained for extended periods with less stringent access controls than production databases.
Remediation:
Sanitize arguments before sending to observability:
const SENSITIVE = /password|secret|token|key|auth|api[-_]?key/i;
function sanitizeArgs(args: any[]): any[] {
return args.slice(0, 5).map(arg => {
if (typeof arg === 'string') {
return arg.substring(0, 500);
}
if (arg && typeof arg === 'object') {
const clean: any = {};
for (const [k, v] of Object.entries(arg).slice(0, 20)) {
clean[k] = SENSITIVE.test(k) ? '[REDACTED]' : v;
}
return clean;
}
return arg;
});
}Apply the same sanitization to the warn method at line 21.
- https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
- https://docs.sentry.io/platforms/javascript/data-management/sensitive-data/
- https://owasp.org/www-community/vulnerabilities/Sensitive_Data_Exposure
🔸 Vulnerability (Warning)
|
|
||
| const initConfig: SentryInitConfig = { | ||
| dsn, | ||
| environment: "production", |
There was a problem hiding this comment.
The environment is hardcoded as "production" instead of using the VITE_SENTRY_ENVIRONMENT variable defined in .env. This causes all errors from development, staging, and production to be tagged as production errors in Sentry, making it difficult to identify actual production issues. Development errors pollute production reports, alerting rules fire incorrectly, and metrics about production error rates become inaccurate.
The .env file defines VITE_SENTRY_ENVIRONMENT=production, indicating environment-specific configuration was intended but not implemented.
Remediation:
Use the environment variable:
const initConfig: SentryInitConfig = {
dsn,
environment: import.meta.env.VITE_SENTRY_ENVIRONMENT ||
(import.meta.env.DEV ? "development" : "production"),
tracesSampleRate,
replay: { sampleRate: replaySampleRate },
enableMetrics: true,
};Ensure your build pipeline sets VITE_SENTRY_ENVIRONMENT appropriately for each deployment environment. Verify in Sentry that events are correctly tagged after this change.
- https://docs.sentry.io/platforms/javascript/configuration/options/#environment
- https://vitejs.dev/guide/env-and-mode.html
- https://12factor.net/config
🔺 Bug (Error)
| try { | ||
| const runtimeContext: RuntimeContext = { | ||
| url: window.location.href, | ||
| userAgent: navigator.userAgent, |
There was a problem hiding this comment.
The context object is spread into Sentry's extra field without validating structure, size, or depth. User-provided or application-generated context could be arbitrarily deep or large, causing stack overflow during serialization, exceeding Sentry's payload limits (typically 200KB), or consuming excessive memory. This creates a DoS vector where errors with extremely large context objects could exhaust resources.
Remediation:
Implement validation for context objects:
function sanitizeContext(obj: any, depth = 0): any {
if (depth > 5) return '[Max depth exceeded]';
if (!obj || typeof obj !== 'object') return obj;
if (Array.isArray(obj)) {
return obj.slice(0, 100).map(item => sanitizeContext(item, depth + 1));
}
const result: any = {};
for (const [key, value] of Object.entries(obj).slice(0, 50)) {
result[key] = sanitizeContext(value, depth + 1);
}
return result;
}
// In captureException:
const sanitized = sanitizeContext(context);
extra: { ...sanitized, url: window.location.href }Consider checking total JSON size and truncating if needed.
- https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html
- https://docs.sentry.io/platforms/javascript/enriching-events/context/
- https://owasp.org/www-community/attacks/Denial_of_Service
🔺 Vulnerability (Error)
| "title": self.status_logger.title, | ||
| }) | ||
| sentry_sdk.capture_exception(e) | ||
| except Exception: |
There was a problem hiding this comment.
The Sentry error reporting is wrapped in a bare except Exception: pass block that silently suppresses all errors during Sentry integration. While preventing Sentry issues from breaking error handling is correct, silent suppression makes it impossible to detect Sentry configuration problems, initialization failures, or network issues. If Sentry is failing, developers have no indication that errors aren't being tracked, defeating the purpose of error tracking.
Remediation:
Replace silent suppression with proper logging:
except BlueprintExecutionError as e:
try:
import sentry_sdk
with sentry_sdk.push_scope() as scope:
scope.set_tag("source", "blueprint_execution")
scope.set_context("blueprint", {
"run_id": self.run_id,
"title": self.status_logger.title,
})
sentry_sdk.capture_exception(e)
except ImportError:
logger.debug("Sentry SDK not available")
except Exception as sentry_error:
logger.warning(f"Failed to report error to Sentry: {sentry_error}", exc_info=True)
self._cancel_all_jobs()
# ... rest of error handlingApply the same pattern to the BaseException handler at line 818.
- https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
- https://docs.python.org/3/library/logging.html
- https://docs.sentry.io/platforms/python/
🔺 Vulnerability (Error)
| scope.set_tag("layer", "server") | ||
| scope.set_tag("source", "event_handler") | ||
| scope.set_tag("event_type", ev.type) | ||
| scope.set_context("event", { |
There was a problem hiding this comment.
The event type (ev.type) is used directly as a Sentry tag without validation. This value originates from frontend messages and user interactions, making it user-controlled input. Using unvalidated user input in observability tags can cause multiple issues: excessively long strings may hit Sentry's tag limits, special characters could interfere with Sentry's query system, and malicious values could pollute your error tracking.
According to OWASP input validation guidelines, all external input must be validated for type, length, format, and range, even when used for logging or observability.
Remediation:
Validate and sanitize event types before using in tags:
import re
def sanitize_tag(value: str, max_len: int = 100) -> str:
if not isinstance(value, str):
value = str(value)
# Allow only alphanumeric, underscore, hyphen, dot
sanitized = re.sub(r'[^a-zA-Z0-9_.-]', '_', value)
return sanitized[:max_len]
# Then use:
scope.set_tag("event_type", sanitize_tag(ev.type))Apply similar sanitization to ev.handler and review the event context (lines 1903-1906) to ensure no sensitive data is included.
- https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html
- https://docs.sentry.io/platforms/python/enriching-events/tags/
- https://cwe.mitre.org/data/definitions/20.html
🔺 Vulnerability (Error)
| sensitive_keys = ["authorization", "cookie", "x-api-key"] | ||
| for key in sensitive_keys: | ||
| headers.pop(key.lower(), None) | ||
| return event |
There was a problem hiding this comment.
The header filtering uses case-sensitive lookup (headers.pop(key.lower(), None)) which only works if header keys are already lowercase. HTTP headers are case-insensitive per RFC 7230, meaning "Authorization", "AUTHORIZATION", and "authorization" are equivalent. If the headers dictionary preserves original casing, filtering will fail, potentially exposing authentication tokens and cookies to Sentry.
This is a critical security issue because authorization headers and cookies contain credentials for impersonating users. The limited scope (only 3 headers) also misses other common authentication headers like "X-Auth-Token" or "X-Session-ID".
Remediation:
Normalize header keys to lowercase before filtering:
def before_send(event, hint):
if "request" in event.get("contexts", {}):
headers = event["contexts"]["request"].get("headers", {})
normalized = {k.lower(): v for k, v in headers.items()}
sensitive = ["authorization", "cookie", "x-api-key",
"x-auth-token", "x-session-id", "x-csrf-token"]
for h in sensitive:
normalized.pop(h, None)
event["contexts"]["request"]["headers"] = normalized
return event- https://datatracker.ietf.org/doc/html/rfc7230#section-3.2
- https://docs.sentry.io/platforms/python/data-management/sensitive-data/
- https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
🔺 Bug (Error)
| "method": request.method, | ||
| "url": str(request.url), | ||
| "path": request.url.path, | ||
| "query_params": dict(request.query_params), |
There was a problem hiding this comment.
Request query parameters are captured and sent to Sentry without filtering sensitive information. Query parameters frequently contain sensitive data like API keys, access tokens, session identifiers, password reset tokens, or PII. This exposes confidential data to anyone with Sentry access and potentially violates data protection regulations (GDPR, CCPA, HIPAA).
The before_send callback filters sensitive headers but doesn't address query parameters, creating an inconsistent security posture. Future features might add sensitive data to URLs that would be automatically logged.
Remediation:
Implement parameter filtering before adding to context:
SENSITIVE_PARAMS = ['token', 'key', 'secret', 'password', 'api_key', 'auth', 'session']
def filter_params(params):
filtered = {}
for k, v in params.items():
if any(p in k.lower() for p in SENSITIVE_PARAMS):
filtered[k] = '[FILTERED]'
else:
filtered[k] = v
return filtered
# In middleware:
"query_params": filter_params(dict(request.query_params))Consider whether query parameters need to be captured at all. The URL path alone is often sufficient for debugging.
- https://docs.sentry.io/platforms/python/data-management/sensitive-data/
- https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html
- https://owasp.org/www-community/vulnerabilities/Sensitive_Data_Exposure
🔺 Privacy/Compliance (Error)
| scope.set_tag("component", "backend") | ||
| scope.set_tag("layer", "server") | ||
| scope.set_tag("source", "blueprint_api_endpoint") | ||
| scope.set_tag("blueprint_id", blueprint_id) |
There was a problem hiding this comment.
The blueprint_id from URL path parameters is used directly as a Sentry tag without validation. URL parameters are user-controlled input that could contain specially crafted values to inject data into your monitoring system. Malicious actors could send requests with extremely long IDs, special characters that interfere with Sentry's indexing, or patterns designed to pollute error tracking.
The OWASP Input Validation Cheat Sheet emphasizes that all external input must be validated at trust boundaries, regardless of intended use.
Remediation:
Validate and sanitize IDs before using as tags:
import re
def sanitize_id(value: str, max_len: int = 64) -> str:
if not value:
return "unknown"
# Remove non-alphanumeric except hyphen/underscore
sanitized = re.sub(r'[^a-zA-Z0-9_-]', '', str(value))
return sanitized[:max_len] if sanitized else "invalid"
# In error handler:
scope.set_tag("blueprint_id", sanitize_id(blueprint_id))
if branch_id:
scope.set_tag("branch_id", sanitize_id(branch_id))If blueprint IDs should be UUIDs, validate explicitly against UUID format. Consider whether these IDs belong in tags or would be better in event context.
- https://cheatsheetseries.owasp.org/cheatsheets/Input_Validation_Cheat_Sheet.html
- https://docs.sentry.io/platforms/python/enriching-events/tags/
- https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/07-Input_Validation_Testing/README
🔸 Vulnerability (Warning)
|
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard. |
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.