From ea7cc23d7bc0b5a854e40cfea6040707c6c622ee Mon Sep 17 00:00:00 2001 From: Anilcan Cakir Date: Fri, 25 Sep 2026 20:05:01 +0300 Subject: [PATCH 1/2] feat: initial magic_sentry plugin Sentry for Magic apps: MagicSentry.run opens one zone for the binding, the .env load, SentryFlutter.init and the app (web needs the outer zone, flutter#100277); an ErrorWidget breadcrumb installer; and SentryServiceProvider, inert without a DSN, which wires sentry_dio (captureFailedRequests off) plus a network interceptor that reports server failures fingerprinted by the normalised endpoint, a user context that follows Auth.stateNotifier (id, email and app-supplied tags), and SentryNavigatorObserver. The install stub publishes the app's own configureSentry; the barrel does not export one. --- .github/workflows/ci.yml | 39 ++++ .github/workflows/publish.yml | 37 ++++ .gitignore | 40 ++++ CHANGELOG.md | 19 ++ CLAUDE.md | 119 ++++++++++ README.md | 71 ++++++ analysis_options.yaml | 13 ++ assets/stubs/install/sentry_config.stub | 86 ++++++++ doc/getting-started/installation.md | 197 +++++++++++++++++ install.yaml | 58 +++++ lib/magic_sentry.dart | 15 ++ lib/src/magic_sentry.dart | 122 +++++++++++ lib/src/sentry_network_interceptor.dart | 197 +++++++++++++++++ lib/src/sentry_service_provider.dart | 114 ++++++++++ lib/src/sentry_user_context.dart | 141 ++++++++++++ pubspec.yaml | 33 +++ test/magic_sentry_test.dart | 142 ++++++++++++ test/sentry_network_interceptor_test.dart | 226 +++++++++++++++++++ test/sentry_service_provider_test.dart | 251 ++++++++++++++++++++++ test/sentry_user_context_test.dart | 181 ++++++++++++++++ 20 files changed, 2101 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/publish.yml create mode 100644 .gitignore create mode 100644 CHANGELOG.md create mode 100644 CLAUDE.md create mode 100644 README.md create mode 100644 analysis_options.yaml create mode 100644 assets/stubs/install/sentry_config.stub create mode 100644 doc/getting-started/installation.md create mode 100644 install.yaml create mode 100644 lib/magic_sentry.dart create mode 100644 lib/src/magic_sentry.dart create mode 100644 lib/src/sentry_network_interceptor.dart create mode 100644 lib/src/sentry_service_provider.dart create mode 100644 lib/src/sentry_user_context.dart create mode 100644 pubspec.yaml create mode 100644 test/magic_sentry_test.dart create mode 100644 test/sentry_network_interceptor_test.dart create mode 100644 test/sentry_service_provider_test.dart create mode 100644 test/sentry_user_context_test.dart diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..79d3826 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,39 @@ +name: CI + +on: + push: + branches: [main, master] + pull_request: + branches: [main, master] + +jobs: + analyze-and-test: + name: Lint & Test + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v7 + + - name: Setup Flutter + uses: subosito/flutter-action@v2 + with: + channel: stable + cache: true + + - name: Install dependencies + run: flutter pub get + + - name: Analyze code + run: flutter analyze --no-fatal-infos + + - name: Check formatting + run: dart format --set-exit-if-changed . + + - name: Run tests + run: flutter test --coverage + + - name: Upload coverage + uses: codecov/codecov-action@v7 + with: + files: coverage/lcov.info + fail_ci_if_error: false diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml new file mode 100644 index 0000000..3b3ac76 --- /dev/null +++ b/.github/workflows/publish.yml @@ -0,0 +1,37 @@ +name: Publish to pub.dev + +on: + push: + tags: + - '[0-9]+.[0-9]+.[0-9]+*' + +jobs: + validate: + name: Validate + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: Setup Flutter + uses: subosito/flutter-action@v2 + with: + channel: stable + cache: true + + - name: Install dependencies + run: flutter pub get + + - name: Analyze code + run: flutter analyze --no-fatal-infos + + - name: Check formatting + run: dart format --set-exit-if-changed . + + - name: Run tests + run: flutter test + + publish: + needs: validate + permissions: + id-token: write # Required for authentication using OIDC + uses: dart-lang/setup-dart/.github/workflows/publish.yml@v1.8.1 diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..94969ca --- /dev/null +++ b/.gitignore @@ -0,0 +1,40 @@ +# Miscellaneous +*.class +*.log +*.pyc +*.swp +.DS_Store +.atom/ +.buildlog/ +.history +.svn/ +migrate_working_dir/ + +# IntelliJ related +*.iml +*.ipr +*.iws +.idea/ + +# VS Code +.vscode/ + +# Test coverage +coverage/ + +# Flutter/Dart/Pub related +**/doc/api/ +**/ios/Flutter/.last_build_id +.dart_tool/ +.flutter-plugins +.flutter-plugins-dependencies +.packages +.pub-cache/ +.pub/ +/build/ +pubspec.lock + +# AI coding tools +.ac/ +# Local development path wiring (never committed; breaks CI which has no sibling paths) +pubspec_overrides.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..6d8f9d2 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,19 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +## [Unreleased] + +### Fixed + +- The barrel (`lib/magic_sentry.dart`) no longer exports `config/sentry.dart`. The published install stub puts the same top-level names (`configureSentry`, `sentryDsn`, `sentryEnabled`, the env key constants) into a consumer app's own `lib/config/sentry.dart`; exporting a package copy alongside that made the documented `main()` an ambiguous import that never compiled. The package no longer carries its own copy of that file; `assets/stubs/install/sentry_config.stub` is now the single source. +- `SentryServiceProvider._registerNetworkInterceptor` no longer swallows a wiring failure when no `log` binding exists. It now always reports: `Log.warning` (with the stack trace) when `log` is bound, `debugPrint` otherwise. +- `SentryUserContext.install()` is now idempotent: a second call on the same instance no longer adds a second listener to `Auth.stateNotifier`, which used to run `apply()` twice per auth-state bump. + +### Added + +- Initial release: `MagicSentry.run` (zone-wrapped `SentryFlutter.init` boot, web-safe per flutter/flutter#100277) and `MagicSentry.installErrorWidgetBreadcrumb` (tags a build error that replaced the interface). +- `SentryServiceProvider`: wires `SentryNetworkInterceptor` into magic's `Http` driver (with `sentry_dio`'s own failed-request capture disabled to avoid double reporting), installs `SentryUserContext` on `Auth.stateNotifier` when a host app supplies `userId`/`userEmail`, and registers `SentryNavigatorObserver` on `MagicRouter`. +- `SentryNetworkInterceptor`: reports a 5xx or a dead connection as an event fingerprinted by the normalised endpoint, a 4xx as a breadcrumb, and deduplicates repeats within a session. +- `SentryUserContext`: keeps Sentry's scope user (id + email) and optional extra tags in step with the host app's own `Authenticatable` model. +- `lib/config/sentry.dart` default `configureSentry`, reading `SENTRY_DSN`, `SENTRY_ENVIRONMENT`, `SENTRY_RELEASE` and `SENTRY_TRACES_SAMPLE_RATE` from `.env`; published to a consumer project via `install.yaml` as an editable starting point. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..c8328f1 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,119 @@ +# Magic Sentry Plugin + +Sentry error and performance monitoring for the Magic Framework. + +**Version:** 0.0.1 · **Dart:** >=3.6.0 · **Flutter:** >=3.27.0 + +## Commands + +**Host-dispatched via `fluttersdk_artisan`'s generic manifest installer**, no +package-specific CLI provider: + +| Command | Description | +|---------|-------------| +| `flutter test --coverage` | Run all tests with coverage | +| `flutter analyze --no-fatal-infos` | Static analysis | +| `dart format .` | Format all code | +| `dart run :artisan plugin:install magic_sentry` | Publish `lib/config/sentry.dart` and inject `SentryServiceProvider` into the consumer's `lib/config/app.dart` | + +## Architecture + +**Pattern**: zone-wrapped boot helper + ServiceProvider, no Driver/Handler chain (there is only one transport, Sentry's own). + +``` +lib/ +├── magic_sentry.dart # Barrel export +└── src/ + ├── magic_sentry.dart # MagicSentry.run + error widget breadcrumb + ├── sentry_service_provider.dart # SentryServiceProvider: wires boot() + ├── sentry_network_interceptor.dart # MagicNetworkInterceptor -> Sentry events/breadcrumbs + └── sentry_user_context.dart # Auth.stateNotifier -> Sentry scope user +install.yaml # Plugin manifest: config publish, provider injection +assets/stubs/install/sentry_config.stub # configureSentry, published to a consumer's + # own lib/config/sentry.dart +``` + +**The barrel exports no config.** `configureSentry`/`sentryDsn`/`sentryEnabled` +and the env key constants live ONLY in the file `plugin:install` publishes into +a consumer app (`lib/config/sentry.dart`, generated from the stub above); the +package itself carries no copy. A package-side export of those same names +would collide with the consumer's own `import 'config/sentry.dart'` the moment +both are imported together, which is exactly what the documented `main()` +does, and `configureSentry` would become an ambiguous import that never +compiles. + +**Why the config is not a magic config factory.** `MagicSentry.run` has to start +Sentry BEFORE `Magic.init` runs, so a build failure during boot is reported +too. At that point `Config` (magic's config repository) is not loaded, so +`configureSentry` reads `.env` directly through `Env`, never through +`Config.get`. `install.yaml` therefore has no `magic.config_factory` key, +unlike a plugin (e.g. `magic_deeplink`) whose config folds into +`Magic.init(configFactories: [...])`. + +**Data flow:** `main()` calls `MagicSentry.run(configure: configureSentry, +appRunner: _boot)` → one zone opens (`runZonedGuarded`) → `WidgetsFlutterBinding +.ensureInitialized()` → `Env.load()` → `SentryFlutter.init(configure, +appRunner: _boot)` → `_boot` runs `Magic.init(...)` then `runApp(...)` → +`SentryServiceProvider.boot()` (registered like any other provider) wires the +network interceptor, the user context, and the navigator observer, all gated +on `Sentry.isEnabled`. + +**User reporting is opt-in and generic.** This package has no model of its +own and never invents a "team" concept: `SentryServiceProvider` takes +`userId`/`userEmail`/`userExtras` callbacks over the host app's own +`Authenticatable` model `T`. Leave them out to skip user reporting; the +network interceptor and navigator observer still wire up. + +## Post-Change Checklist + +After ANY source code change, sync **before committing**: + +1. **`CHANGELOG.md`**: Add entry under `[Unreleased]` section +2. **`README.md`**: Update if features, API, or usage changes +3. **`doc/`**: Update relevant documentation files + +## Development Flow (TDD) + +Every feature, fix, or refactor must go through the red-green-refactor cycle: + +1. **Red**: Write a failing test that describes the expected behavior +2. **Green**: Write the minimum code to make the test pass +3. **Refactor**: Clean up while keeping tests green + +**Rules:** +- No production code without a failing test first +- Run `flutter test` after every change: all tests must stay green +- Run `dart analyze` after every change: zero warnings, zero errors +- Run `dart format .` before committing: zero formatting issues + +## Testing + +- Mock via contract inheritance (no mockito): a `NetworkDriver` test double + `implements NetworkDriver` and throws `UnimplementedError` on members the + test does not exercise, rather than a generated mock. +- `MagicApp.reset(); Magic.flush();` in `setUp`, the house pattern also used + by `magic` itself, so a previous test's singleton never leaks into the next. +- `MagicRouter.reset()` around any test that calls `addObserver`: it is a + process-global singleton with no per-test isolation otherwise. +- A test that observes Sentry's scope (the user, a tag, a breadcrumb) calls + `Sentry.init` with a syntactically valid but unreachable DSN first + (`https://public@o0.ingest.sentry.io/0`); nothing in this package's tests + ever sends a real event, so no network call happens regardless. + +## Key Gotchas + +| Mistake | Fix | +|---------|-----| +| Reading `sendDefaultPii`/session replay from `.env` | Never do this; both stay hardcoded off in `configureSentry`. An app that wants either overrides its own `configure` callback explicitly | +| `sentry_dio`'s `addSentry()` AND `SentryNetworkInterceptor` both capturing failed requests | `SentryServiceProvider` disables `captureFailedRequests` on `addSentry()`; `SentryNetworkInterceptor` is the only layer that decides event vs breadcrumb | +| Calling `MagicRouter.instance.addObserver` after `routerConfig` was built | Register `SentryServiceProvider` in `lib/config/app.dart`'s `providers` list, which boots before any route is built | +| A guarded network/log call throwing anyway | `Magic.bound('log')`/`try`/`catch` around `app.make('network')`, matching magic_deeplink's pattern for an optional dependency that must not take the whole boot down | +| Team logic, tags, or breadcrumbs specific to one app | Does not belong here; use `SentryServiceProvider`'s `userExtras` callback instead | + +## Skills & Extensions + +- `fluttersdk:magic-framework`: Magic Framework patterns: facades, service providers, IoC, Eloquent ORM. Use for ANY code touching Magic APIs. + +## CI + +- `ci.yml`: push/PR → `flutter pub get` → `flutter analyze --no-fatal-infos` → `dart format --set-exit-if-changed` → `flutter test --coverage` → codecov upload diff --git a/README.md b/README.md new file mode 100644 index 0000000..62b4134 --- /dev/null +++ b/README.md @@ -0,0 +1,71 @@ +# Magic Sentry Plugin + +Sentry error and performance monitoring for the [Magic Framework](https://magic.fluttersdk.com). Wraps `sentry_flutter` boot in a web-safe zone, reports the HTTP failures magic's `Http` facade swallows into `MagicResponse` values, keeps Sentry's scope user in step with `Auth.stateNotifier`, and registers a navigator observer on `MagicRouter`. + +**Version:** 0.0.1 · **Dart:** >=3.6.0 · **Flutter:** >=3.27.0 + +## Install + +```bash +flutter pub add magic_sentry +dart run :artisan plugin:install magic_sentry +``` + +`plugin:install` (from `fluttersdk_artisan`, no package-specific CLI needed) reads this package's `install.yaml`, publishes `lib/config/sentry.dart` (documents the `.env` keys read at boot: `SENTRY_DSN`, `SENTRY_ENVIRONMENT`, `SENTRY_RELEASE`, `SENTRY_TRACES_SAMPLE_RATE`), and injects `SentryServiceProvider` into `lib/config/app.dart`'s provider list. See [doc/getting-started/installation.md](doc/getting-started/installation.md) for the full walkthrough, including the `main.dart` wiring the installer cannot do for you. + +## Quick start + +```dart +// main.dart +import 'config/sentry.dart'; + +void main() { + MagicSentry.run(configure: configureSentry, appRunner: _boot); +} + +Future _boot() async { + MagicSentry.installErrorWidgetBreadcrumb(); + await Magic.init(configFactories: [...]); + runApp(MyApp()); +} +``` + +```dart +// lib/config/app.dart +(app) => SentryServiceProvider( + app, + userId: (user) => user.id, + userEmail: (user) => user.email, +), +``` + +## Architecture + +``` +lib/ +├── magic_sentry.dart # Barrel export +└── src/ + ├── magic_sentry.dart # MagicSentry.run + error widget breadcrumb + ├── sentry_service_provider.dart # SentryServiceProvider: wires boot() + ├── sentry_network_interceptor.dart # MagicNetworkInterceptor -> Sentry events/breadcrumbs + └── sentry_user_context.dart # Auth.stateNotifier -> Sentry scope user +install.yaml # Plugin manifest: config publish, provider injection +assets/stubs/install/sentry_config.stub # configureSentry, published to a consumer's + # own lib/config/sentry.dart; the package + # itself carries no copy of it (see below) +``` + +The barrel does not export `configureSentry`/`sentryDsn`/`sentryEnabled`: those +names live only in the file `plugin:install` publishes into a consumer app +(`lib/config/sentry.dart`, from the stub above). A package copy exported +alongside it would make the documented `main()` (which imports both the +barrel and that local file) an ambiguous import. + +## Key decisions + +- **Not enabled by default.** `sentryEnabled`/`Sentry.isEnabled` answer false with an empty `SENTRY_DSN`, and every integration in this package degrades to a no-op rather than throwing. +- **`sendDefaultPii` and session replay are never turned on by this package.** An app that wants either does so explicitly in its own `configure` callback. +- **HTTP double-reporting is avoided on purpose.** `SentryServiceProvider` disables `sentry_dio`'s own `captureFailedRequests`, since `SentryNetworkInterceptor` is the layer that decides event vs breadcrumb. +- **User reporting is opt-in and generic.** This package has no model of its own; pass `userId`/`userEmail` (and optionally `userExtras`) for your own `Authenticatable` model, or leave them out to skip user reporting entirely. + +See [doc/getting-started/installation.md](doc/getting-started/installation.md) for configuration details. diff --git a/analysis_options.yaml b/analysis_options.yaml new file mode 100644 index 0000000..767640d --- /dev/null +++ b/analysis_options.yaml @@ -0,0 +1,13 @@ +include: package:flutter_lints/flutter.yaml + +analyzer: + errors: + unintended_html_in_doc_comment: ignore + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** diff --git a/assets/stubs/install/sentry_config.stub b/assets/stubs/install/sentry_config.stub new file mode 100644 index 0000000..f636c80 --- /dev/null +++ b/assets/stubs/install/sentry_config.stub @@ -0,0 +1,86 @@ +import 'package:magic/magic.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// The `.env` key naming this app's Sentry DSN. Empty or absent disables +/// the SDK entirely (see [sentryDsn]). +const String sentryDsnEnvKey = 'SENTRY_DSN'; + +/// The `.env` key naming the reported environment (`production`, `staging`, +/// ...). Defaults to `local` when absent, which keeps a developer machine's +/// events out of a production environment filter. +const String sentryEnvironmentEnvKey = 'SENTRY_ENVIRONMENT'; + +/// The `.env` key naming the release identifier attached to every event. +/// +/// Left empty by default: the SDK derives `package@version+build` from the +/// platform manifest on iOS and Android, but Flutter web has no manifest to +/// read, so a web build needs this set explicitly (typically via +/// `--dart-define` at build time) for release health to mean anything. +const String sentryReleaseEnvKey = 'SENTRY_RELEASE'; + +/// The `.env` key naming the fraction of transactions that are traced. +const String sentryTracesSampleRateEnvKey = 'SENTRY_TRACES_SAMPLE_RATE'; + +/// The default traces sample rate when [sentryTracesSampleRateEnvKey] is +/// absent from `.env`. +const double sentryDefaultTracesSampleRate = 0.2; + +/// Whether this build reports to Sentry at all. +/// +/// True exactly when [sentryDsn] is non-empty. Kept alongside [sentryDsn] +/// rather than reading `Sentry.isEnabled`: this answers the question BEFORE +/// `SentryFlutter.init` has run, which [configureSentry] needs to decide +/// whether the rest of its options matter. +bool get sentryEnabled => sentryDsn.isNotEmpty; + +/// The DSN read from `.env`, or an EMPTY STRING when absent. +/// +/// The return type is deliberately `String` and never `String?`. `Sentry.init` +/// throws `ArgumentError('DSN is required.')` when the DSN is null, while an +/// empty string is a documented, supported "do not send anything" (see +/// `SentryOptions.dsn`). Since `appRunner` runs inside `SentryFlutter.init`, +/// a nullable DSN would stop the whole app from booting on every machine that +/// has not set [sentryDsnEnvKey], including CI and the test suite. +String get sentryDsn => Env.getString(sentryDsnEnvKey, ''); + +/// The default `configure` callback for `MagicSentry.run`. +/// +/// Reads the standard `.env` keys documented above and applies them to +/// [options]. Pass this directly when the standard keys are all an app +/// needs: +/// +/// ```dart +/// await MagicSentry.run(configure: configureSentry, appRunner: _boot); +/// ``` +/// +/// This file was generated once by `plugin:install` from +/// `magic_sentry`'s own install manifest, and is yours to edit from here: an +/// app that needs more (a computed sample rate, a release gating rule) +/// changes this function directly rather than writing a second one. +void configureSentry(SentryFlutterOptions options) { + options.dsn = sentryDsn; + options.environment = Env.getString(sentryEnvironmentEnvKey, 'local'); + + final String release = Env.getString(sentryReleaseEnvKey, ''); + + if (release.isNotEmpty) { + options.release = release; + } + + options.tracesSampleRate = Env.get( + sentryTracesSampleRateEnvKey, + sentryDefaultTracesSampleRate, + ); + + // Fixed rather than read from `.env`: an app that wants PII sent has to + // override this here rather than flip a single environment variable, + // since a stray value there would otherwise leak a user's IP address and + // request bodies to every event. + options.sendDefaultPii = false; + + // Release health depends on named routes: `SentryNavigatorObserver` reads + // `RouteSettings.name`, so a router that leaves pages unnamed reports zero + // sessions while the transport keeps working. See + // `doc/getting-started/installation.md#release-health`. + options.enableAutoSessionTracking = true; +} diff --git a/doc/getting-started/installation.md b/doc/getting-started/installation.md new file mode 100644 index 0000000..c387150 --- /dev/null +++ b/doc/getting-started/installation.md @@ -0,0 +1,197 @@ +# Installation + +- [Introduction](#introduction) +- [Requirements](#requirements) +- [Installing the Package](#installing-the-package) +- [Running the Install Command](#running-the-install-command) +- [Wiring main()](#wiring-main) +- [Registering the Service Provider](#registering-the-service-provider) +- [Configuration Reference](#configuration-reference) +- [Release Health](#release-health) +- [Next Steps](#next-steps) + + +## Introduction + +`magic_sentry` wires [Sentry](https://sentry.io) into a Magic application: a +web-safe zone-wrapped boot, HTTP failure reporting for magic's `Http` facade +(which never throws), a scope user kept in step with `Auth.stateNotifier`, +and a navigator observer on `MagicRouter`. + +Sentry has to start BEFORE `Magic.init`, so a failure during boot is reported +too. That is the one place this package's install differs from a typical +Magic plugin: there is no `magic.config_factory` entry, because `Config` is +not loaded yet at the point Sentry needs its DSN. + + +## Requirements + +- **Dart SDK**: 3.6.0 or higher +- **Flutter**: 3.27.0 or higher +- **Magic Framework** installed and bootstrapped (`lib/config/app.dart` present) + + +## Installing the Package + +```bash +flutter pub add magic_sentry +``` + +Or add it manually to `pubspec.yaml`: + +```yaml +dependencies: + magic_sentry: ^0.0.1 +``` + +Then fetch dependencies: + +```bash +flutter pub get +``` + + +## Running the Install Command + +This package ships no package-specific CLI provider; `fluttersdk_artisan`'s +generic manifest installer reads its `install.yaml` directly: + +```bash +dart run :artisan plugin:install magic_sentry +``` + +The command performs the following steps: + +1. **Creates** `lib/config/sentry.dart` from the bundled stub. +2. **Injects** `SentryServiceProvider` into the `providers` list in `lib/config/app.dart`. + +> [!NOTE] +> Steps 3 and 4 of a typical Magic plugin install (a config-factory injection +> into `Magic.init`, and a native platform edit) do not apply here: this +> package has neither. `main()` wiring is a manual step, covered next. + + +## Wiring main() + +`MagicSentry.run` replaces the whole body of `main()`. It opens one zone, +initialises Flutter's binding, loads `.env`, then starts Sentry and hands +control to your existing boot function as `appRunner`: + +```dart +import 'package:flutter/material.dart'; +import 'package:magic/magic.dart'; +import 'package:magic_sentry/magic_sentry.dart'; +import 'config/app.dart'; +import 'config/sentry.dart'; // injected by install + +void main() { + MagicSentry.run(configure: configureSentry, appRunner: _boot); +} + +Future _boot() async { + // Tags a build error that replaced the interface, distinguishing it from + // an exception that merely got logged. Call it before the first widget + // builds. + MagicSentry.installErrorWidgetBreadcrumb(); + + await Magic.init( + configFactories: [ + () => appConfig, + ], + ); + + runApp(MagicApplication(title: 'My App')); +} +``` + +`configureSentry` comes from the published `lib/config/sentry.dart` and reads +`.env` directly (see [Configuration Reference](#configuration-reference) +below). Write your own `configure` callback instead when the standard keys +are not enough. + +> [!IMPORTANT] +> `MagicSentry.run` must be the ENTIRE body of `main()`. On Flutter web, +> `SentryFlutter.init` opens its own zone only when called from the root zone +> (`PlatformDispatcher.onError` alone misses async errors there, see +> [flutter/flutter#100277](https://github.com/flutter/flutter/issues/100277)). +> Entering the zone in `MagicSentry.run` first is what avoids a second, +> competing zone and the "Zone mismatch" error Flutter raises when the +> binding and `runApp` end up in different zones. + + +## Registering the Service Provider + +If you ran `plugin:install`, the provider was already injected with no type +argument and no user callbacks, which is a valid, working default (the +network interceptor and the navigator observer wire up; user reporting is +skipped). Edit `lib/config/app.dart` to add your own user model: + +```dart +import 'package:magic/magic.dart'; +import 'package:magic_sentry/magic_sentry.dart'; // injected by install +import '../app/models/user.dart'; + +final appConfig = { + 'app': { + 'providers': [ + (app) => RouteServiceProvider(app), + (app) => AppServiceProvider(app), + (app) => SentryServiceProvider( + app, + userId: (user) => user.id, + userEmail: (user) => user.email, + // Optional: anything else worth tagging once a user is reported. + userExtras: (user) => { + if (user.currentTeam?.id case final String teamId) + 'team_id': teamId, + }, + ), // injected by install, edit the type argument and callbacks by hand + ], + }, +}; +``` + +> [!TIP] +> `SentryServiceProvider.boot()` returns immediately when `Sentry.isEnabled` +> is false, which is every local run and the whole test suite unless a test +> calls `Sentry.init` itself. Nothing it wires ever runs without a DSN. + + +## Configuration Reference + +The generated `lib/config/sentry.dart` reads these `.env` keys: + +| Key | Default | Purpose | +|-----|---------|---------| +| `SENTRY_DSN` | `''` (disabled) | Your project's DSN. Empty leaves the SDK inert; `appRunner` still runs. | +| `SENTRY_ENVIRONMENT` | `'local'` | Tags every event, e.g. `production`, `staging`. | +| `SENTRY_RELEASE` | `''` (unset) | Attached to every event. Required on web, which has no package manifest to auto-detect a release from; set it via `--dart-define` at build time. | +| `SENTRY_TRACES_SAMPLE_RATE` | `0.2` | Fraction of transactions traced. Client spans continue into the API through `sentry_dio`'s propagated `sentry-trace` header, so raising this multiplies server spans too. | + +Two things `configureSentry` does NOT read from `.env`, on purpose: + +- **`sendDefaultPii` stays `false`.** Flipping it sends the user's IP address + and request bodies with every event. An app that needs it enables it in + its own `configure` callback, explicitly, rather than through a stray + environment variable. +- **Session replay is never enabled.** It has no Flutter web implementation + and its own PII review; add it yourself if your app needs it. + + +## Release Health + +`configureSentry` turns on `enableAutoSessionTracking`, but a session is only +recorded when `SentryNavigatorObserver` sees a NAMED route change (it reads +`RouteSettings.name`). `SentryServiceProvider` registers the observer for +you; make sure your own routes carry a `name`, or release health silently +stays empty while error reporting keeps working. + + +## Next Steps + +- **HTTP reporting**: `SentryNetworkInterceptor` reports a 5xx or a dead + connection (`MagicError.statusCode == 0`) as an event, a 4xx as a + breadcrumb, and deduplicates repeats within a session. See + `lib/src/sentry_network_interceptor.dart`. +- **User context**: `SentryUserContext` follows `Auth.stateNotifier`; see + `lib/src/sentry_user_context.dart` for the id/email/extras contract. diff --git a/install.yaml b/install.yaml new file mode 100644 index 0000000..d00dae8 --- /dev/null +++ b/install.yaml @@ -0,0 +1,58 @@ +# magic_sentry install manifest (PluginInstaller DSL schema v1). +# +# Sentry starts BEFORE Magic.init (see MagicSentry.run in +# lib/src/magic_sentry.dart), so unlike magic_deeplink's config this package's +# published config is NOT a magic config-factory map: `Config` is not loaded +# yet at the point it is read. Install publishes a plain Dart file exposing +# `configureSentry`/`sentryDsn`/`sentryEnabled`, which main() imports and +# passes to `MagicSentry.run(configure: ...)` directly, by hand: there is no +# `magic.config_factory` key here because there is nothing for Magic.init to +# fold it into. +# +# `SentryServiceProvider` IS injected into the provider list (it wires the +# network interceptor, the user context and the navigator observer once +# Magic.init has run), so `magic.provider` is set. +# +# See doc/install_yaml_schema.md (in fluttersdk_artisan) for the full schema. + +plugin_name: magic_sentry + +publish: + install/sentry_config: lib/config/sentry.dart + +magic: + provider: SentryServiceProvider + +post_install: + message: | + magic_sentry installed. Bootstrap workflow: + + 1. Review lib/config/sentry.dart and confirm the `.env` keys it reads + (SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_RELEASE, + SENTRY_TRACES_SAMPLE_RATE) match what your deploy pipeline sets. + + 2. Wrap main() in MagicSentry.run, BEFORE Magic.init: + + import 'config/sentry.dart'; + + void main() { + MagicSentry.run(configure: configureSentry, appRunner: _boot); + } + + Future _boot() async { + await Magic.init(configFactories: [...]); + runApp(MyApp()); + } + + 3. SentryServiceProvider was added to lib/config/app.dart's providers + list. Pass userId/userEmail (and optionally userExtras) if you want + Sentry's scope user kept in step with Auth.stateNotifier: + + (app) => SentryServiceProvider( + app, + userId: (user) => user.id, + userEmail: (user) => user.email, + ), + + Leave the type argument and both callbacks out to skip user reporting; + the network interceptor and the navigator observer wire up either way. diff --git a/lib/magic_sentry.dart b/lib/magic_sentry.dart new file mode 100644 index 0000000..ddd5d13 --- /dev/null +++ b/lib/magic_sentry.dart @@ -0,0 +1,15 @@ +/// Sentry error and performance monitoring for the Magic Framework. +/// +/// This barrel deliberately does NOT export a `configureSentry`/`sentryDsn`/ +/// `sentryEnabled` set: `install.yaml` publishes that as a plain Dart file +/// into a consumer app's own `lib/config/sentry.dart` (see +/// `assets/stubs/install/sentry_config.stub`), and the documented `main()` +/// imports both this barrel and that local file. A package copy of the same +/// top-level names here would make `configureSentry` an ambiguous import the +/// moment both are imported together, which never compiles. +library; + +export 'src/magic_sentry.dart'; +export 'src/sentry_network_interceptor.dart'; +export 'src/sentry_service_provider.dart'; +export 'src/sentry_user_context.dart'; diff --git a/lib/src/magic_sentry.dart b/lib/src/magic_sentry.dart new file mode 100644 index 0000000..e86c51d --- /dev/null +++ b/lib/src/magic_sentry.dart @@ -0,0 +1,122 @@ +import 'dart:async'; + +import 'package:flutter/widgets.dart'; +import 'package:magic/magic.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// Boots Sentry, then the app, inside one zone. +/// +/// Call this as the whole body of `main()`, before `Magic.init`: +/// +/// ```dart +/// void main() { +/// MagicSentry.run(configure: configureSentry, appRunner: _boot); +/// } +/// +/// Future _boot() async { +/// await Magic.init(configFactories: [...]); +/// runApp(MyApp()); +/// } +/// ``` +/// +/// `Config` is not loaded yet at this point, so [configure] cannot read a +/// `Magic` config map; it reads `.env` directly (via `Env`, which [run] loads +/// before calling [SentryFlutter.init]). `lib/config/sentry.dart`'s +/// `configureSentry` is a ready-made [configure] that does exactly this. +class MagicSentry { + MagicSentry._(); + + /// Opens one zone for Flutter's binding, `.env` loading, Sentry's own + /// initialisation and the whole app, then hands zone-level errors to + /// Sentry. + /// + /// ONE ZONE FOR EVERYTHING is the point, not an implementation detail: on + /// Flutter web `SentryFlutter.init` opens its own `runZonedGuarded` only + /// when called from the ROOT zone (`PlatformDispatcher.onError` alone does + /// not catch every async error there, see flutter/flutter#100277). Entering + /// a zone here first makes that check false, so `SentryFlutter.init` calls + /// [appRunner] in THIS zone instead of a second one, and the binding + /// [appRunner] eventually initialises is never asked to serve a callback + /// registered in a different zone. + /// + /// [configure] receives the `SentryFlutterOptions` to fill in (DSN, + /// environment, sample rates); [appRunner] is everything the app does + /// after that, typically `Magic.init(...)` followed by `runApp(...)`. An + /// empty DSN inside [configure] leaves the SDK inert and [appRunner] still + /// runs: see `lib/config/sentry.dart`'s `sentryDsn`. + static void run({ + required FutureOr Function(SentryFlutterOptions options) configure, + required FutureOr Function() appRunner, + }) { + runZonedGuarded( + () async { + WidgetsFlutterBinding.ensureInitialized(); + + // Loaded before Sentry: the DSN lives in `.env`, and `configure` + // would otherwise run before it exists. `Env.load` guards on a + // static `_isLoaded` flag and returns immediately once set, so a + // host that also calls it from `appRunner` (e.g. because + // `Magic.init` reads `.env` too) pays no real cost for the repeat + // call; that same guard also means a DIFFERENT `fileName` passed to + // that later call is silently ignored rather than merged, since the + // file this loaded is the one that sticks for the rest of the run. + await Env.load(); + + await SentryFlutter.init(configure, appRunner: appRunner); + }, + (Object error, StackTrace stackTrace) async { + // Both halves of what Sentry's own zone handler does: report it, then + // dump it to the console. Without the dump an uncaught async error + // would vanish from the local console the moment this zone took over. + await Sentry.captureException(error, stackTrace: stackTrace); + + FlutterError.dumpErrorToConsole( + FlutterErrorDetails(exception: error, stack: stackTrace), + forceReport: true, + ); + }, + ); + } + + /// Marks a build error that replaced the whole interface with Sentry's + /// `fatal` breadcrumb level, before the app's own screen ever renders. + /// + /// Flutter already reports the underlying exception through + /// `FlutterError.onError`, which the SDK hooks on its own; what this adds is + /// a tag distinguishing "an exception got logged" from "an exception + /// replaced the interface with `ErrorWidget`'s flat grey box", which + /// otherwise look identical in the issue list. Call once, early in + /// `appRunner`, before the first widget builds: + /// + /// ```dart + /// Future _boot() async { + /// MagicSentry.installErrorWidgetBreadcrumb(); + /// await Magic.init(...); + /// runApp(MyApp()); + /// } + /// ``` + /// + /// The default widget is returned unchanged: a replacement screen would + /// need colours, fonts and a theme, each a thing that can throw in the + /// exact situation this runs in. + static void installErrorWidgetBreadcrumb() { + final ErrorWidgetBuilder defaultErrorWidget = ErrorWidget.builder; + + ErrorWidget.builder = (FlutterErrorDetails details) { + Sentry.addBreadcrumb( + Breadcrumb( + category: 'ui.error_widget', + message: 'a build error replaced the interface', + level: SentryLevel.fatal, + data: { + 'library': details.library, + 'context': details.context?.toString(), + 'exception': details.exception.toString(), + }, + ), + ); + + return defaultErrorWidget(details); + }; + } +} diff --git a/lib/src/sentry_network_interceptor.dart b/lib/src/sentry_network_interceptor.dart new file mode 100644 index 0000000..fa69e0a --- /dev/null +++ b/lib/src/sentry_network_interceptor.dart @@ -0,0 +1,197 @@ +import 'package:flutter/foundation.dart' show visibleForTesting; +import 'package:magic/magic.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// What a failed HTTP call becomes in Sentry. +enum SentryHttpDisposition { + /// An issue someone should look at. + event, + + /// Context attached to whatever fails next, with no issue of its own. + breadcrumb, +} + +/// Reports HTTP failures that magic's `Http` facade would otherwise swallow. +/// +/// WHY THIS EXISTS AT ALL. magic's `Http` facade never throws: every call +/// returns a `MagicResponse` and the caller reads `response.successful`, so a +/// failure is a VALUE, not an exception. A global error handler therefore sees +/// none of them, and a host app's own habit is usually to log the failure and +/// return quietly. Without this interceptor Sentry reports almost nothing +/// about the surface that actually breaks. +/// +/// TWO DECISIONS, both of which a test pins: +/// +/// 1. WHAT DESERVES AN ISSUE. A 5xx or a dead connection is a fault. A 4xx is +/// usually the product working correctly: 401 is an expired session, 422 is +/// a form the user filled in wrong, 404 is a record someone else deleted. +/// Filing those as issues is how an inbox becomes one nobody opens, so they +/// become breadcrumbs and provide context for the next real failure instead. +/// 2. HOW IT GROUPS. The fingerprint carries a NORMALISED path, because a raw +/// one contains a record id and would open a separate issue per record. +/// One broken endpoint arriving as a thousand issues looks exactly like a +/// thousand bugs. +/// +/// It reports and returns; it never changes the outcome of a request. Wire it +/// in with [SentryServiceProvider], which also disables `sentry_dio`'s own +/// failed-request capture to avoid reporting the same failure twice. +class SentryNetworkInterceptor extends MagicNetworkInterceptor { + /// Failures already reported this session, as `METHOD /path {status}`. + /// + /// Session-scoped rather than time-windowed on purpose: a web session is a + /// tab, so it ends when the page does, and a returning user reports the + /// failure again. A timer would need a clock, a test seam and a decision + /// about what "recently" means, to save an issue that Sentry already groups. + @visibleForTesting + static final Set reportedFailures = {}; + + /// Report the failure, then hand it back untouched. + /// + /// The return value is meaningful to magic: a `MagicResponse` RESOLVES the + /// failure as a success, anything else lets it continue down the chain. This + /// returns exactly what it was given, so observability can never change + /// behaviour. + /// + /// Nothing here checks whether Sentry is enabled. It does not need to: with + /// no DSN the SDK is inert and every call below is a no-op, which is the + /// state every local run and the whole test suite are in. + @override + dynamic onError(MagicError error) { + final int status = error.statusCode; + final String method = error.request?.method.toUpperCase() ?? 'UNKNOWN'; + final String endpoint = normalizeEndpoint(error.request?.url ?? 'unknown'); + + if (dispositionFor(status) == SentryHttpDisposition.breadcrumb) { + Sentry.addBreadcrumb( + Breadcrumb( + type: 'http', + category: 'http', + level: SentryLevel.warning, + message: '$method $endpoint -> $status', + data: { + 'method': method, + 'endpoint': endpoint, + 'status_code': status, + }, + ), + ); + + return error; + } + + // One issue per distinct failure per session, then breadcrumbs. Without + // this a client polling any endpoint is a flood source rather than a + // signal: a one-hour backend outage across a couple of hundred open tabs + // is tens of thousands of identical events, and most Sentry plans bill by + // event count with no overage budget. + // + // The first occurrence is the one that carries information; the rest are + // the same fingerprint arriving repeatedly, which Sentry would group into + // one issue anyway while still billing for each. Their breadcrumbs still + // show the retry pattern to whoever opens that issue. + if (!reportedFailures.add('$method $endpoint $status')) { + Sentry.addBreadcrumb( + Breadcrumb( + type: 'http', + category: 'http', + level: SentryLevel.error, + message: '$method $endpoint -> $status (repeat)', + data: { + 'method': method, + 'endpoint': endpoint, + 'status_code': status, + }, + ), + ); + + return error; + } + + Sentry.captureEvent( + SentryEvent( + level: SentryLevel.error, + logger: 'http', + message: SentryMessage( + '$method $endpoint failed with $status', + template: '%s %s failed with %s', + params: [method, endpoint, status], + ), + // The status comes LAST so a single endpoint's 500 and its 502 stay + // separate issues: they usually have different causes (the app threw + // vs. the gateway could not reach it). + fingerprint: ['http', method, endpoint, '$status'], + // A structured context rather than `extra`, which the SDK deprecated + // in favour of exactly this. Deliberately no request body and no + // headers: an app that turns `sendDefaultPii` on is opting into that + // risk itself, this interceptor does not assume it. + contexts: Contexts() + ..['http_failure'] = { + 'method': method, + 'endpoint': endpoint, + 'status_code': status, + if (error.message != null) 'transport_message': error.message, + }, + ), + ); + + return error; + } + + /// Whether a status code is worth an issue or only a breadcrumb. + /// + /// [statusCode] is `MagicError.statusCode`, which answers 0 when there is no + /// response at all. That zero is the most important case in the function: it + /// is a timeout, a refused connection or a DNS failure, and on web it is also + /// what a CORS rejection looks like. + @visibleForTesting + static SentryHttpDisposition dispositionFor(int statusCode) { + if (statusCode == 0) { + return SentryHttpDisposition.event; + } + + if (statusCode >= 500) { + return SentryHttpDisposition.event; + } + + return SentryHttpDisposition.breadcrumb; + } + + /// Replace identifier segments with `{id}` so one endpoint is one issue. + /// + /// [url] is the request path as magic recorded it, which may carry a query + /// string. The query goes entirely: `?page=3` would otherwise split one + /// endpoint's failures across every page a user happened to be on. + @visibleForTesting + static String normalizeEndpoint(String url) { + final String path = url.split('?').first; + + return path + .split('/') + .map((segment) => _looksLikeIdentifier(segment) ? '{id}' : segment) + .join('/'); + } + + /// Whether a path segment is an identifier rather than a route name. + /// + /// Both numeric and UUID primary keys are covered, and only those two, + /// since those are the two shapes a magic-backed API issues. + /// + /// The UUID pattern is fully anchored rather than a loose "long and hex-ish" + /// test, because a loose one collapses real route names: `acknowledged` and + /// `deactivate` are both long, and hex-adjacent enough to fool a sloppy + /// matcher. + static bool _looksLikeIdentifier(String segment) { + if (segment.isEmpty) { + return false; + } + + if (RegExp(r'^\d+$').hasMatch(segment)) { + return true; + } + + return RegExp( + r'^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-' + r'[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$', + ).hasMatch(segment); + } +} diff --git a/lib/src/sentry_service_provider.dart b/lib/src/sentry_service_provider.dart new file mode 100644 index 0000000..aad913f --- /dev/null +++ b/lib/src/sentry_service_provider.dart @@ -0,0 +1,114 @@ +import 'package:flutter/foundation.dart' show debugPrint; +import 'package:magic/magic.dart'; +import 'package:sentry_dio/sentry_dio.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +import 'sentry_network_interceptor.dart'; +import 'sentry_user_context.dart'; + +/// Wires Sentry into a magic application, after `Magic.init` has run. +/// +/// Everything here is inert without a DSN: [boot] returns immediately when +/// `Sentry.isEnabled` answers false, which is every local run and the whole +/// test suite unless a test calls `Sentry.init` itself. Register it in +/// `lib/config/app.dart`'s `providers` list; `MagicSentry.run` is what starts +/// the SDK itself, earlier, in `main()` (`Sentry.isEnabled` is only ever true +/// by the time this provider's `boot()` runs because that already happened). +/// +/// [T] is the host app's own user model. Pass [userId] and [userEmail] to +/// have this provider install a [SentryUserContext] that follows +/// `Auth.stateNotifier`; leave them null to skip user reporting entirely +/// (the network interceptor and the navigator observer still wire up either +/// way). [userExtras] adds anything else the app wants tagged on every event +/// (a team id, a plan) once a user is reported. +class SentryServiceProvider extends ServiceProvider { + /// Creates the provider. + /// + /// [userId] and [userEmail] read the reported id/email straight off [T]; + /// both are required together, since a scope user with no id is not a + /// useful one. [userExtras] is optional and additive. + SentryServiceProvider( + super.app, { + String Function(T user)? userId, + String? Function(T user)? userEmail, + Map Function(T user)? userExtras, + }) : _userContext = (userId != null && userEmail != null) + ? SentryUserContext( + id: userId, + email: userEmail, + extras: userExtras, + ) + : null; + + /// The user-context installer this provider wires in [boot], or null when + /// the host app declined to supply [userId]/[userEmail]. + final SentryUserContext? _userContext; + + @override + void register() { + // Nothing to bind: this provider only wires existing services together. + } + + @override + Future boot() async { + if (!Sentry.isEnabled) { + return; + } + + _registerNetworkInterceptor(); + _userContext?.install(); + + // Registered here rather than in `main()` because `MagicRouter` refuses + // an observer once it has built its `routerConfig`, and that build + // happens on the first frame; `boot()` runs before that. Without this + // every event is filed against a route this app never names, because + // magic drives go_router and the SDK cannot see through it. + MagicRouter.instance.addObserver(SentryNavigatorObserver()); + } + + /// Attach Sentry to the network driver, in two layers. + /// + /// `addSentry()` (from `sentry_dio`) contributes the automatic half: an + /// HTTP breadcrumb and a span per request, so a captured failure arrives + /// with the calls that preceded it. `captureFailedRequests` is turned OFF + /// because its events are raised inside the sentry_dio adapter, which means + /// their stack trace points at the SDK rather than at the caller, and they + /// cannot carry the endpoint-based fingerprint [SentryNetworkInterceptor] + /// needs; leaving it on would also double-report every failure. + /// + /// [SentryNetworkInterceptor] is the second layer and does the actual + /// reporting, from magic's own interceptor chain where the real call site + /// is still on the stack. It runs after `addSentry()` so its events carry + /// the breadcrumbs that layer just recorded. + /// + /// The driver is resolved from the container rather than constructed, so a + /// host that never registered one (a bare widget test) would throw here and + /// take the whole boot with it. Reporting is not worth failing a boot over, + /// so the failure degrades to a log line rather than a rethrow. It is + /// caught NARROWLY (this `try` wraps only the wiring, not the rest of + /// [boot]) and ALWAYS reported: `Log.warning` when a `log` binding exists, + /// `debugPrint` otherwise, so a host with no logging configured yet still + /// sees the failure on its console instead of it disappearing. + void _registerNetworkInterceptor() { + try { + final NetworkDriver driver = app.make('network'); + + if (driver is DioNetworkDriver) { + driver.configureDriver( + (dio) => dio.addSentry(captureFailedRequests: false), + ); + } + + driver.addInterceptor(SentryNetworkInterceptor()); + } catch (error, stackTrace) { + const String message = + '[sentry] Could not wire Sentry into the network driver'; + + if (Magic.bound('log')) { + Log.warning('$message: $error', {'stackTrace': stackTrace.toString()}); + } else { + debugPrint('$message: $error\n$stackTrace'); + } + } + } +} diff --git a/lib/src/sentry_user_context.dart b/lib/src/sentry_user_context.dart new file mode 100644 index 0000000..5b9acf1 --- /dev/null +++ b/lib/src/sentry_user_context.dart @@ -0,0 +1,141 @@ +import 'package:flutter/foundation.dart' show ValueNotifier, visibleForTesting; +import 'package:magic/magic.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// Keeps Sentry's scope user in step with `Auth.stateNotifier`. +/// +/// The host app supplies [id] and [email] extractors for its own user model +/// [T], plus an optional [extras] callback for anything else worth tagging on +/// every event (a team id, a plan, whatever the app's own domain needs). +/// This package has no opinion on what [T] is or what belongs in [extras]; +/// it only owns the WHEN. +/// +/// ## Kept in sync by the auth state, not by each call site +/// +/// [install] subscribes to `Auth.stateNotifier`, which fires on login, on +/// logout, on a session restore at boot and (in an app with teams) on a team +/// switch. Doing it there rather than at each of those call sites is what +/// keeps a future one correct for free, and it is also what guarantees the +/// LOGOUT case: a scope that keeps the previous user attached would file the +/// next visitor's errors under the person who signed out, which is worse +/// than having no user at all. +class SentryUserContext { + /// Creates a context that reads [T] through [id] and [email]. + /// + /// [id] and [email] should read straight off the model (no network calls, + /// no side effects): they run on every auth state change. [extras] is + /// called only when a user is reported, and its keys REPLACE whatever the + /// previous call set, so the same key can carry a new value across a team + /// switch without leaking the old one. + SentryUserContext({ + required String Function(T user) id, + required String? Function(T user) email, + Map Function(T user)? extras, + }) : _id = id, + _email = email, + _extras = extras; + + /// Extracts the reported user id from [T]. Empty means "no user". + final String Function(T user) _id; + + /// Extracts the reported email from [T], or null when the account has none + /// (a guest account, typically). + final String? Function(T user) _email; + + /// Extracts extra scope tags from [T], applied only when a user is + /// reported. + final Map Function(T user)? _extras; + + /// The tag keys the last [apply] set, so the next call can clear exactly + /// those rather than guessing at what [_extras] might have named. + Set _extraTagKeys = {}; + + /// The notifier [install] last subscribed [apply] to, or null before the + /// first call. + /// + /// Identity-compared rather than a boolean latch: `Auth.stateNotifier` + /// itself can change across an app's lifetime (a test that fakes a new + /// guard between installs, for instance), so a plain "already installed" + /// flag would refuse to resubscribe to a genuinely new notifier and this + /// would silently stop following auth state instead of double-subscribing. + ValueNotifier? _installedOn; + + /// Start following the auth state, and apply whatever it says right now. + /// + /// The immediate call matters: `Auth.restore()` typically runs during boot, + /// so by the time this is installed there is often already a signed-in + /// user, and waiting for the next change would leave the whole first + /// session anonymous. + /// + /// Safe to call more than once on the same instance: a repeat is a no-op + /// past the immediate [apply] (still run, since it only reasserts the + /// current state) rather than a second subscription. Without the guard, a + /// second `install()` on an unchanged notifier would leave [apply] running + /// TWICE per future auth-state bump, once for each subscription. + void install() { + apply(); + + final ValueNotifier notifier = Auth.stateNotifier; + + if (identical(_installedOn, notifier)) { + return; + } + + _installedOn?.removeListener(apply); + _installedOn = notifier..addListener(apply); + } + + /// Push the current auth state into Sentry's scope. + /// + /// A no-op in practice when the SDK has no DSN, which is every debug run. + void apply() { + final bool signedIn = Auth.check(); + final T? user = signedIn ? Auth.user() : null; + final SentryUser? reported = userFor(user); + + Sentry.configureScope((scope) { + // Explicitly null on sign-out. See the class docblock: a stale user is + // worse than none. + scope.setUser(reported); + + for (final String key in _extraTagKeys) { + scope.removeTag(key); + } + _extraTagKeys = {}; + + if (reported == null || user == null || _extras == null) { + return; + } + + final Map extras = _extras(user); + + extras.forEach(scope.setTag); + _extraTagKeys = extras.keys.toSet(); + }); + } + + /// The Sentry user for [user], or null when there is nobody to report. + /// + /// Null for an ABSENT user and equally for one whose [_id] answers the + /// empty string, since several magic-backed apps model "no session" as an + /// empty user object rather than a null one. + @visibleForTesting + SentryUser? userFor(T? user) { + if (user == null) { + return null; + } + + final String id = _id(user); + + if (id.isEmpty) { + return null; + } + + final String? email = _email(user); + + return SentryUser( + id: id, + email: (email != null && email.isNotEmpty) ? email : null, + ); + } +} diff --git a/pubspec.yaml b/pubspec.yaml new file mode 100644 index 0000000..6a7817f --- /dev/null +++ b/pubspec.yaml @@ -0,0 +1,33 @@ +name: magic_sentry +description: Sentry error and performance monitoring for the Magic Framework. +version: 0.0.1 +homepage: https://magic.fluttersdk.com/sentry +documentation: https://magic.fluttersdk.com/packages/sentry/getting-started/installation +repository: https://github.com/fluttersdk/magic_sentry +issue_tracker: https://github.com/fluttersdk/magic_sentry/issues + +topics: + - sentry + - error-monitoring + - observability + - magic + - flutter + +environment: + sdk: ">=3.6.0 <4.0.0" + flutter: ">=3.27.0" + +dependencies: + flutter: + sdk: flutter + # Auth.stateNotifier and MagicRouter.instance.addObserver both land at + # 0.0.21; see the published API check under doc/getting-started/installation.md. + magic: ^0.0.21 + sentry_flutter: ^9.27.0 + sentry_dio: ^9.27.0 + +dev_dependencies: + flutter_test: + sdk: flutter + test: ^1.25.0 + flutter_lints: ^6.0.0 diff --git a/test/magic_sentry_test.dart b/test/magic_sentry_test.dart new file mode 100644 index 0000000..97fe6ee --- /dev/null +++ b/test/magic_sentry_test.dart @@ -0,0 +1,142 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:magic_sentry/magic_sentry.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// Locks [MagicSentry.run]'s contract: one zone, Sentry started first, the +/// app's own boot function always runs regardless of whether a DSN was +/// supplied, and a zone-level error is reported rather than crashing the +/// isolate. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('barrel exports', () { + test('the package does not export the app-local sentry config', () { + // The install stub (`assets/stubs/install/sentry_config.stub`) + // publishes a top-level `configureSentry` into a consumer app's own + // `lib/config/sentry.dart`, which the documented `main()` imports + // alongside `package:magic_sentry/magic_sentry.dart`. Re-exporting a + // package copy of the same name from the barrel turns that `main()` + // into an ambiguous import that never compiles, which is exactly what + // shipped once. There is no reflection API to ask a compiled library + // what it exported, so this reads the barrel's own source instead: the + // closest thing to a compile-time contract this package can pin + // without publishing a second package just to import both at once. + final String barrel = File('lib/magic_sentry.dart').readAsStringSync(); + + expect( + barrel, + isNot(contains("'config/sentry.dart'")), + reason: 'the published install stub is the single source for ' + 'configureSentry/sentryDsn/sentryEnabled; the barrel must not ' + 'also export a package copy of the same names.', + ); + }); + }); + + group('MagicSentry.run', () { + tearDown(() async { + if (Sentry.isEnabled) { + await Sentry.close(); + } + }); + + test('an empty DSN leaves Sentry disabled and still runs appRunner', + () async { + final completer = Completer(); + + MagicSentry.run( + configure: (options) => options.dsn = '', + appRunner: () { + completer.complete(); + }, + ); + + await completer.future.timeout(const Duration(seconds: 5)); + + expect(Sentry.isEnabled, isFalse); + }); + + test('a non-empty DSN enables Sentry before appRunner runs', () async { + final completer = Completer(); + + MagicSentry.run( + configure: (options) => + options.dsn = 'https://public@o0.ingest.sentry.io/0', + appRunner: () { + completer.complete(Sentry.isEnabled); + }, + ); + + expect( + await completer.future.timeout(const Duration(seconds: 5)), + isTrue, + ); + }); + + test('a zone error from appRunner does not crash the isolate', () async { + // There is no bound transport here to assert Sentry actually SENT the + // error, so this pins the weaker but load-bearing half: the handler + // this wraps `SentryFlutter.init` in does not rethrow, so the test + // (and a real app booted through MagicSentry.run) survives an + // uncaught async error instead of taking the isolate down with it. + final completer = Completer(); + + MagicSentry.run( + configure: (options) => options.dsn = '', + appRunner: () { + scheduleMicrotask(() => throw StateError('boom')); + completer.complete(); + }, + ); + + await completer.future.timeout(const Duration(seconds: 5)); + }); + }); + + group('installErrorWidgetBreadcrumb', () { + late ErrorWidgetBuilder original; + + setUp(() { + original = ErrorWidget.builder; + }); + + tearDown(() async { + ErrorWidget.builder = original; + if (Sentry.isEnabled) { + await Sentry.close(); + } + }); + + test('tags a build error and still renders the default widget', () async { + await Sentry.init( + (options) => options.dsn = 'https://public@o0.ingest.sentry.io/0', + ); + + MagicSentry.installErrorWidgetBreadcrumb(); + + final details = FlutterErrorDetails( + exception: StateError('layout exploded'), + library: 'magic_sentry test', + ); + + final Widget widget = ErrorWidget.builder(details); + + expect(widget, isA()); + + List breadcrumbs = const []; + await Sentry.configureScope((scope) => breadcrumbs = scope.breadcrumbs); + + expect( + breadcrumbs.any( + (b) => + b.category == 'ui.error_widget' && b.level == SentryLevel.fatal, + ), + isTrue, + ); + }); + }); +} diff --git a/test/sentry_network_interceptor_test.dart b/test/sentry_network_interceptor_test.dart new file mode 100644 index 0000000..c7ad4f2 --- /dev/null +++ b/test/sentry_network_interceptor_test.dart @@ -0,0 +1,226 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:magic/magic.dart'; +import 'package:magic_sentry/magic_sentry.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// Locks the two decisions that separate a useful Sentry inbox from an +/// abandoned one. +/// +/// magic's HTTP layer never throws: `Http.get()` hands back a +/// `MagicResponse` and the caller reads `response.successful`, so a failure is +/// a VALUE. Nothing reaches Sentry unless this interceptor puts it there, +/// which makes both decisions below load-bearing rather than cosmetic. +/// +/// The first is what deserves an issue at all. A 401 is a session that +/// expired and a 422 is a form the user filled in wrong: both are the +/// product working, and filing them as issues is how an inbox becomes +/// something nobody opens. A 500 or a dead connection is the opposite. +/// +/// The second is grouping. Sentry groups by fingerprint, and a fingerprint +/// carrying a raw path means `/monitors/` opens a NEW issue for every +/// record a caller owns. One broken endpoint would then arrive as a thousand +/// separate issues, which is indistinguishable from a thousand separate bugs. +void main() { + group('dispositionFor', () { + test('a dead connection earns an issue', () { + // `MagicError.statusCode` answers 0 when there is no response at all, + // which is the shape a timeout or a refused connection arrives in. + expect( + SentryNetworkInterceptor.dispositionFor(0), + SentryHttpDisposition.event, + ); + }); + + test('a server fault earns an issue', () { + expect( + SentryNetworkInterceptor.dispositionFor(500), + SentryHttpDisposition.event, + ); + expect( + SentryNetworkInterceptor.dispositionFor(503), + SentryHttpDisposition.event, + ); + }); + + test('an expected client answer is only a breadcrumb', () { + for (final status in [400, 401, 403, 404, 409, 422, 429]) { + expect( + SentryNetworkInterceptor.dispositionFor(status), + SentryHttpDisposition.breadcrumb, + reason: + '$status is the product working, not a fault to page anyone about.', + ); + } + }); + }); + + group('normalizeEndpoint', () { + test('it collapses a numeric id so one endpoint is one issue', () { + expect( + SentryNetworkInterceptor.normalizeEndpoint('/monitors/123/checks'), + '/monitors/{id}/checks', + ); + }); + + test('it collapses a uuid', () { + expect( + SentryNetworkInterceptor.normalizeEndpoint( + '/monitors/9f8e7d6c-5b4a-4321-8765-0fedcba98765/metrics', + ), + '/monitors/{id}/metrics', + ); + }); + + test('it leaves a static path alone', () { + expect( + SentryNetworkInterceptor.normalizeEndpoint( + '/dashboard/active-incidents'), + '/dashboard/active-incidents', + ); + }); + + test('it does not mistake a word for an id', () { + // A segment being long or hex-ish is not enough; `incidents` must + // survive. + expect( + SentryNetworkInterceptor.normalizeEndpoint('/incidents/acknowledged'), + '/incidents/acknowledged', + ); + }); + + test('it drops a query string, which would explode grouping', () { + expect( + SentryNetworkInterceptor.normalizeEndpoint( + '/monitors?page=3&per_page=50'), + '/monitors', + ); + }); + }); + + group('deduplication', () { + setUp(SentryNetworkInterceptor.reportedFailures.clear); + + test('the same failure is only reported once per session', () { + // A client that polls, retries, or reconnects turns a single ongoing + // failure into hundreds of identical reports if nothing deduplicates: + // Sentry groups them anyway, so the extras cost quota for no new + // information. + final interceptor = SentryNetworkInterceptor(); + final error = MagicError(message: 'gateway down'); + + interceptor.onError(error); + interceptor.onError(error); + interceptor.onError(error); + + expect(SentryNetworkInterceptor.reportedFailures, hasLength(1)); + }); + + test('a different endpoint or status is its own report', () { + final interceptor = SentryNetworkInterceptor(); + + interceptor.onError(MagicError(message: 'a')); + interceptor.onError( + MagicError( + message: 'b', + request: MagicRequest(url: '/monitors', method: 'GET'), + ), + ); + + expect(SentryNetworkInterceptor.reportedFailures, hasLength(2)); + }); + }); + + group('captured events', () { + setUp(SentryNetworkInterceptor.reportedFailures.clear); + + tearDown(() async { + if (Sentry.isEnabled) { + await Sentry.close(); + } + }); + + test('a 500 becomes an event fingerprinted on the normalised endpoint', + () async { + SentryEvent? captured; + + await Sentry.init((options) { + options.dsn = 'https://public@o0.ingest.sentry.io/0'; + // Read the event Sentry would have sent, then block the send: this + // test proves what gets CAPTURED, never anything reaching a real + // network. + options.beforeSend = (event, hint) { + captured = event; + return null; + }; + }); + + SentryNetworkInterceptor().onError( + MagicError( + message: 'server exploded', + request: MagicRequest(url: '/monitors/123', method: 'GET'), + response: MagicResponse( + data: null, + statusCode: 500, + headers: const {}, + ), + ), + ); + + // `Sentry.captureEvent` runs asynchronously inside the SDK's own hub; + // the interceptor fires it and returns immediately (see `onError`'s + // own docs on why: reporting must never change the outcome of a + // request). Draining the event queue is what lets `beforeSend` run + // before this test asserts on it. + await pumpEventQueue(); + + expect(captured, isNotNull); + expect(captured!.fingerprint, ['http', 'GET', '/monitors/{id}', '500']); + }); + + test('a 404 produces no event, only a breadcrumb', () async { + bool sendAttempted = false; + + await Sentry.init((options) { + options.dsn = 'https://public@o0.ingest.sentry.io/0'; + options.beforeSend = (event, hint) { + sendAttempted = true; + return event; + }; + }); + + SentryNetworkInterceptor().onError( + MagicError( + message: 'not found', + request: MagicRequest(url: '/monitors/123', method: 'GET'), + response: MagicResponse( + data: null, + statusCode: 404, + headers: const {}, + ), + ), + ); + + await pumpEventQueue(); + + expect(sendAttempted, isFalse); + }); + }); + + group('onError', () { + setUp(SentryNetworkInterceptor.reportedFailures.clear); + + test('it returns the error untouched so the interceptor chain survives', + () { + // magic reads the return value: a `MagicResponse` RESOLVES the failure + // as a success, anything else lets it continue. Reporting must never + // change the outcome of a request, so this returns exactly what it was + // handed. + final error = MagicError(message: 'boom'); + + expect( + identical(SentryNetworkInterceptor().onError(error), error), + isTrue, + ); + }); + }); +} diff --git a/test/sentry_service_provider_test.dart b/test/sentry_service_provider_test.dart new file mode 100644 index 0000000..7909c75 --- /dev/null +++ b/test/sentry_service_provider_test.dart @@ -0,0 +1,251 @@ +import 'package:flutter/foundation.dart' show DebugPrintCallback, debugPrint; +import 'package:flutter_test/flutter_test.dart'; +import 'package:magic/magic.dart'; +import 'package:magic_sentry/magic_sentry.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +class _TestUser extends Model with Authenticatable { + _TestUser({required String id}) { + setRawAttributes({'id': id}); + } + + @override + String get table => 'users'; + + @override + String get resource => 'users'; + + @override + String get id => getAttribute('id') as String? ?? ''; +} + +/// Records every interceptor handed to it. Every other [NetworkDriver] member +/// is unused by [SentryServiceProvider] and left unimplemented on purpose, +/// matching this codebase's "mock via contract inheritance" convention +/// (no mockito) rather than a reflection-driven double. +class _SpyNetworkDriver implements NetworkDriver { + final List interceptors = []; + + @override + void addInterceptor(MagicNetworkInterceptor interceptor) { + interceptors.add(interceptor); + } + + @override + Future index( + String resource, { + Map? filters, + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future show( + String resource, + String id, { + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future store( + String resource, + Map data, { + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future update( + String resource, + String id, + Map data, { + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future destroy( + String resource, + String id, { + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future get( + String url, { + Map? query, + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future post( + String url, { + dynamic data, + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future put( + String url, { + dynamic data, + Map? headers, + }) => + throw UnimplementedError(); + + @override + Future delete(String url, {Map? headers}) => + throw UnimplementedError(); + + @override + Future upload( + String url, { + required Map data, + required Map files, + Map? headers, + }) => + throw UnimplementedError(); +} + +/// Locks what [SentryServiceProvider.boot] wires, and when. +/// +/// Everything it does is gated on `Sentry.isEnabled`, which is false in +/// every group here except "with a DSN": that split is the test for the +/// disabled path, since a provider that wired anything without a DSN would +/// touch a container binding (`network`) a bare test never set up. +void main() { + group('SentryServiceProvider', () { + setUp(() { + MagicApp.reset(); + Magic.flush(); + MagicRouter.reset(); + }); + + tearDown(() async { + Auth.unfake(); + MagicRouter.reset(); + if (Sentry.isEnabled) { + await Sentry.close(); + } + }); + + test('does nothing without a DSN', () async { + final provider = SentryServiceProvider(MagicApp.instance); + + await provider.boot(); + + expect(MagicRouter.instance.observers, isEmpty); + }); + + group('with a DSN', () { + setUp(() async { + await Sentry.init( + (options) => options.dsn = 'https://public@o0.ingest.sentry.io/0', + ); + }); + + test('wires the network interceptor into the bound driver', () async { + final driver = _SpyNetworkDriver(); + Magic.app.setInstance('network', driver); + + await SentryServiceProvider(MagicApp.instance).boot(); + + expect(driver.interceptors, hasLength(1)); + expect(driver.interceptors.single, isA()); + }); + + test('registers the navigator observer', () async { + Magic.app.setInstance('network', _SpyNetworkDriver()); + + await SentryServiceProvider(MagicApp.instance).boot(); + + expect( + MagicRouter.instance.observers.whereType(), + hasLength(1), + ); + }); + + test('installs the user context when id/email callbacks are supplied', + () async { + Magic.app.setInstance('network', _SpyNetworkDriver()); + Auth.fake(user: _TestUser(id: 'usr_1')); + + await SentryServiceProvider<_TestUser>( + MagicApp.instance, + userId: (user) => user.id, + userEmail: (user) => null, + ).boot(); + + SentryUser? reported; + await Sentry.configureScope((scope) => reported = scope.user); + + expect(reported, isNotNull); + expect(reported!.id, 'usr_1'); + }); + + test('skips user reporting when no callbacks are supplied', () async { + Magic.app.setInstance('network', _SpyNetworkDriver()); + + await SentryServiceProvider(MagicApp.instance).boot(); + + SentryUser? reported; + await Sentry.configureScope((scope) => reported = scope.user); + + expect(reported, isNull); + }); + + test( + 'degrades to a debugPrint report, not silence, with no log bound ' + 'and no network driver bound', () async { + // Nothing bound at 'network': `app.make` throws inside the provider, + // and boot() must not propagate it, since reporting is not worth + // failing the whole app boot over. But it must not go silent either: + // with no `log` binding (the state of a bare test, and any host that + // has not registered logging yet) the fallback is `debugPrint`, so + // the failure still reaches a console instead of vanishing. + final List printed = []; + final DebugPrintCallback original = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + if (message != null) printed.add(message); + }; + + try { + await SentryServiceProvider(MagicApp.instance).boot(); + } finally { + debugPrint = original; + } + + expect( + MagicRouter.instance.observers.whereType(), + hasLength(1), + ); + expect( + printed.any((line) => line.contains('sentry')), + isTrue, + reason: 'the failure to wire the network interceptor must be ' + 'reported, not swallowed', + ); + }); + + test( + 'reports through Log.warning, with a stack trace, when log is ' + 'bound', () async { + final FakeLogManager log = Log.fake(); + + try { + await SentryServiceProvider(MagicApp.instance).boot(); + } finally { + Log.unfake(); + } + + log.assertLoggedCount(1); + expect(log.entries.single.level, 'warning'); + expect(log.entries.single.message, contains('sentry')); + expect(log.entries.single.context, contains('stackTrace')); + }); + }); + }); +} diff --git a/test/sentry_user_context_test.dart b/test/sentry_user_context_test.dart new file mode 100644 index 0000000..65190be --- /dev/null +++ b/test/sentry_user_context_test.dart @@ -0,0 +1,181 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:magic/magic.dart'; +import 'package:magic_sentry/magic_sentry.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// A minimal `Authenticatable` model standing in for a host app's own user. +/// +/// Hydrated through `setRawAttributes` (not `fill`) so a test can set `id` +/// without declaring it `fillable`, mirroring how a real model hydrates from +/// an API response rather than from a form. +class _TestUser extends Model with Authenticatable { + _TestUser({required String id, String? email, String? teamId}) { + setRawAttributes({ + 'id': id, + if (email != null) 'email': email, + if (teamId != null) 'team_id': teamId, + }); + } + + @override + String get table => 'users'; + + @override + String get resource => 'users'; + + @override + String get id => getAttribute('id') as String? ?? ''; + + String? get email => getAttribute('email') as String?; + + String? get teamId => getAttribute('team_id') as String?; +} + +SentryUserContext<_TestUser> _context({ + Map Function(_TestUser user)? extras, +}) { + return SentryUserContext<_TestUser>( + id: (user) => user.id, + email: (user) => user.email, + extras: extras, + ); +} + +/// Reads the scope user Sentry currently holds, the only way to observe +/// [SentryUserContext.apply]'s effect without sending a real event. +Future _scopeUser() async { + SentryUser? user; + await Sentry.configureScope((scope) => user = scope.user); + return user; +} + +Future> _scopeTags() async { + Map tags = const {}; + await Sentry.configureScope((scope) => tags = scope.tags); + return tags; +} + +/// Locks what an issue reports about WHO hit it: an id, an email and +/// whatever [SentryUserContext]'s `extras` callback names. Nothing else. +/// +/// The empty cases are the ones worth pinning: an `Authenticatable` whose id +/// extractor answers the empty string is treated as "no session", since +/// several magic-backed apps model a signed-out state as an empty model +/// rather than a null one. +void main() { + group('userFor', () { + test('it reports nobody when there is no session', () { + expect(_context().userFor(null), isNull); + }); + + test('it reports nobody for a user with an empty id', () { + expect(_context().userFor(_TestUser(id: '')), isNull); + }); + + test('it carries the id and the email', () { + final reported = _context().userFor( + _TestUser(id: 'usr_42', email: 'operator@example.test'), + ); + + expect(reported, isNotNull); + expect(reported!.id, 'usr_42'); + expect(reported.email, 'operator@example.test'); + }); + + test('it omits an email the user does not have', () { + final reported = _context().userFor(_TestUser(id: 'usr_9')); + + expect(reported, isNotNull); + expect(reported!.id, 'usr_9'); + expect(reported.email, isNull); + }); + }); + + group('install/apply against a live scope', () { + setUp(() async { + MagicApp.reset(); + Magic.flush(); + // A DSN is required to bind a real hub (`Sentry.isEnabled == false` + // would leave `configureScope` a no-op), but nothing here sends an + // event, so no request ever reaches it. + await Sentry.init( + (options) => options.dsn = 'https://public@o0.ingest.sentry.io/0'); + }); + + tearDown(() async { + Auth.unfake(); + await Sentry.close(); + }); + + test('login sets the scope user id and email', () async { + Auth.fake(user: _TestUser(id: 'usr_1', email: 'a@b.test')); + + _context().install(); + + final reported = await _scopeUser(); + expect(reported, isNotNull); + expect(reported!.id, 'usr_1'); + expect(reported.email, 'a@b.test'); + }); + + test('logout clears the scope user', () async { + Auth.fake(user: _TestUser(id: 'usr_1', email: 'a@b.test')); + + _context().install(); + await Auth.logout(); + + expect(await _scopeUser(), isNull); + }); + + test('extras are tagged once a user is reported', () async { + Auth.fake(user: _TestUser(id: 'usr_1', teamId: 'team_9')); + + _context( + extras: (user) => user.teamId != null ? {'team_id': user.teamId!} : {}, + ).install(); + + expect(await _scopeTags(), containsPair('team_id', 'team_9')); + }); + + test('extras are cleared on logout rather than left stale', () async { + Auth.fake(user: _TestUser(id: 'usr_1', teamId: 'team_9')); + + _context( + extras: (user) => user.teamId != null ? {'team_id': user.teamId!} : {}, + ).install(); + await Auth.logout(); + + expect(await _scopeTags(), isNot(contains('team_id'))); + }); + + test('installing twice applies once per bump, not twice', () async { + // A second `install()` on the same instance used to add a second + // listener with no removal of the first, so every future auth-state + // bump ran `apply()` (and thus this `id` extractor) twice instead of + // once. `id` is called on every `apply()` that sees a signed-in user + // (`userFor` reads it unconditionally), which makes it a direct count + // of how many times `apply()` actually ran for ONE bump. + Auth.fake(user: _TestUser(id: 'usr_1')); + + int idCalls = 0; + final context = SentryUserContext<_TestUser>( + id: (user) { + idCalls++; + return user.id; + }, + email: (user) => user.email, + ); + + context.install(); + context.install(); + idCalls = 0; // discard the two immediate applies `install()` itself ran + + // Bumped directly rather than through another `Auth.fake()` call: a + // second fake swaps in a brand new notifier, which would prove nothing + // about a listener registered on the notifier that already exists. + Auth.stateNotifier.value++; + + expect(idCalls, 1); + }); + }); +} From a05c02f1a69a0c7a41dab598a661e9f800477d26 Mon Sep 17 00:00:00 2001 From: Anilcan Cakir Date: Fri, 25 Sep 2026 20:13:42 +0300 Subject: [PATCH 2/2] fix: report the user from userId alone, email is optional --- CLAUDE.md | 5 +++-- README.md | 2 +- lib/src/sentry_service_provider.dart | 20 +++++++++++--------- test/sentry_service_provider_test.dart | 17 +++++++++++++++++ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c8328f1..bdca9aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,8 +61,9 @@ on `Sentry.isEnabled`. **User reporting is opt-in and generic.** This package has no model of its own and never invents a "team" concept: `SentryServiceProvider` takes `userId`/`userEmail`/`userExtras` callbacks over the host app's own -`Authenticatable` model `T`. Leave them out to skip user reporting; the -network interceptor and navigator observer still wire up. +`Authenticatable` model `T`; `userId` alone turns reporting on (email is +optional). Leave `userId` out to skip user reporting; the network interceptor +and navigator observer still wire up. ## Post-Change Checklist diff --git a/README.md b/README.md index 62b4134..c399933 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,6 @@ barrel and that local file) an ambiguous import. - **Not enabled by default.** `sentryEnabled`/`Sentry.isEnabled` answer false with an empty `SENTRY_DSN`, and every integration in this package degrades to a no-op rather than throwing. - **`sendDefaultPii` and session replay are never turned on by this package.** An app that wants either does so explicitly in its own `configure` callback. - **HTTP double-reporting is avoided on purpose.** `SentryServiceProvider` disables `sentry_dio`'s own `captureFailedRequests`, since `SentryNetworkInterceptor` is the layer that decides event vs breadcrumb. -- **User reporting is opt-in and generic.** This package has no model of its own; pass `userId`/`userEmail` (and optionally `userExtras`) for your own `Authenticatable` model, or leave them out to skip user reporting entirely. +- **User reporting is opt-in and generic.** This package has no model of its own; pass `userId` (plus `userEmail` when your model has one, and optionally `userExtras`) for your own `Authenticatable` model, or leave `userId` out to skip user reporting entirely. See [doc/getting-started/installation.md](doc/getting-started/installation.md) for configuration details. diff --git a/lib/src/sentry_service_provider.dart b/lib/src/sentry_service_provider.dart index aad913f..5fe360a 100644 --- a/lib/src/sentry_service_provider.dart +++ b/lib/src/sentry_service_provider.dart @@ -15,33 +15,35 @@ import 'sentry_user_context.dart'; /// the SDK itself, earlier, in `main()` (`Sentry.isEnabled` is only ever true /// by the time this provider's `boot()` runs because that already happened). /// -/// [T] is the host app's own user model. Pass [userId] and [userEmail] to -/// have this provider install a [SentryUserContext] that follows -/// `Auth.stateNotifier`; leave them null to skip user reporting entirely +/// [T] is the host app's own user model. Pass [userId] (and [userEmail] when +/// the model has one) to have this provider install a [SentryUserContext] +/// that follows `Auth.stateNotifier`; leave [userId] null to skip user +/// reporting entirely /// (the network interceptor and the navigator observer still wire up either /// way). [userExtras] adds anything else the app wants tagged on every event /// (a team id, a plan) once a user is reported. class SentryServiceProvider extends ServiceProvider { /// Creates the provider. /// - /// [userId] and [userEmail] read the reported id/email straight off [T]; - /// both are required together, since a scope user with no id is not a - /// useful one. [userExtras] is optional and additive. + /// [userId] turns user reporting on: a scope user with no id is not a + /// useful one, so without it nothing is reported. [userEmail] is optional, + /// because a model with no email (a guest-only app) still deserves an id + /// on its events. [userExtras] is optional and additive. SentryServiceProvider( super.app, { String Function(T user)? userId, String? Function(T user)? userEmail, Map Function(T user)? userExtras, - }) : _userContext = (userId != null && userEmail != null) + }) : _userContext = userId != null ? SentryUserContext( id: userId, - email: userEmail, + email: userEmail ?? (T user) => null, extras: userExtras, ) : null; /// The user-context installer this provider wires in [boot], or null when - /// the host app declined to supply [userId]/[userEmail]. + /// the host app supplied no [userId]. final SentryUserContext? _userContext; @override diff --git a/test/sentry_service_provider_test.dart b/test/sentry_service_provider_test.dart index 7909c75..ab6c793 100644 --- a/test/sentry_service_provider_test.dart +++ b/test/sentry_service_provider_test.dart @@ -186,6 +186,23 @@ void main() { expect(reported!.id, 'usr_1'); }); + test('reports the user from userId alone, for a model with no email', + () async { + Magic.app.setInstance('network', _SpyNetworkDriver()); + Auth.fake(user: _TestUser(id: 'usr_2')); + + await SentryServiceProvider<_TestUser>( + MagicApp.instance, + userId: (user) => user.id, + ).boot(); + + SentryUser? reported; + await Sentry.configureScope((scope) => reported = scope.user); + + expect(reported?.id, 'usr_2'); + expect(reported?.email, isNull); + }); + test('skips user reporting when no callbacks are supplied', () async { Magic.app.setInstance('network', _SpyNetworkDriver());