[PM-XXXXX] fix: Stop a failed event write from failing its caller

[withinfocus]

🎟️ Tracking

Follows https://bitwarden.atlassian.net/wiki/spaces/EN/pages/3406037038/How+Authentication+Is+Coupled+to+Audit+Event+Delivery. Related to #8364, which covered dead letter retention on the integration tier. This one covers the producer side of the event tier.

📔 Objective

A failed event write went straight to its caller. When the event-logging topic hit its quota during incident 044, AzureServiceBusService.PublishEventAsync threw, nothing caught it, and /connect/token returned a 500 for every user on every client. The event was lost either way, so the exception preserved nothing and only cost the request.

IEventWriteService now resolves to a NonThrowingEventWriteService that counts and logs a failed write instead of throwing it. One wrapper at the chokepoint covers all 149 event call sites and any added later.

Two registrations opt out on purpose:

• The keyed "persistent" registration the listeners use stays bare. EventRepositoryHandler has to see a failure so the message is redelivered rather than silently acked.
• The Events host passes surfaceWriteFailures: true. Recording the event is what a request to /collect is for, and the events it carries (client vault exports, copied passwords, viewed ciphers) have no server-side equivalent, so returning 200 on a dropped write would lose them. Clients buffer and retry on a 500.

RepositoryEventWriteService is wrapped, so self-hosted now drops an audit write rather than failing the request. Review flagged this: nothing retries behind that path, and OTLP export is off by default on self-hosted, leaving the error log as the only signal. I kept it wrapped because a self-hosted login should not fail on an Event insert, and the database is already a hard dependency of login, so the practical change is small. Worth a second opinion.

Two counters on the Bitwarden.Events meter replace the signal the 500s were giving us: bitwarden.events.write_failures per failed operation and bitwarden.events.dropped per event lost. Both carry the exception type, and the dropped counter also carries the event type, so a responder can tell whether authentication events were in a lost batch. EventType is a closed enum and carries no organization, user, or vault data.

Additional setup

Two things this needs that are not in the diff.

Alerting on the counters. Collection and OTLP export happen automatically, since Bitwarden.Server.Sdk already registers the Bitwarden.* meter prefix, so nothing needs configuring to get the data flowing. What is missing is an alert. A sustained nonzero rate on either counter means we are dropping audit events. Someone needs to own that alert before this merges, because the PR removes the signal we have now. Today a failed publish shows up as a 500 rate on /connect/token. After this change it shows up nowhere unless the counter is being watched.

Service Bus alerting, out of scope here but needed before we can call 044 closed: topic size as a percentage of quota, which would have caught the saturation well before logins started failing, and subscription lag or oldest message age against whatever recovery window we settle on. Neither is a code change.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the NonThrowingEventWriteService decorator, the EventWriteMetrics counters, the AddEventWriteService<T> registration helper, the Events host opt-out, and the README/mermaid updates. Traced every IEventWriteService consumer: production writes reach it only through IEventService, both listener handlers resolve the keyed "persistent" registration that stays unwrapped, AzureQueueHostedService constructs RepositoryEventWriteService directly rather than through DI, and WebApplicationFactoryBase replaces the interface descriptor outright — so no redelivery path is masked, and the Events host never calls AddDefaultServices, so its surfaceWriteFailures: true registration cannot be shadowed by a TryAdd from the shared path. Confirmed TryAddSingleton<T>() preserves container-owned disposal of EventIntegrationEventWriteService.DisposeAsync, that CreateManyAsync materializes inside the try so a deferred sequence that throws mid-enumeration is counted rather than escaping, and that the tags (exception.type, event.type) and the error log carry only a closed enum and an exception type — no user, organization, or vault data.

On the self-hosted question the description raises: wrapping RepositoryEventWriteService reads as the right call, because the write targets the same database the surrounding request already depends on, so an insert failure is a mode in which the operation was unlikely to complete anyway — and the counters plus the Serilog file sink leave an operator more signal than a failed login did. The alerting owner named under "Additional setup" is the load-bearing follow-up here, since those counters are the only replacement for the /connect/token 500 rate this removes.

Code Review Details

No code findings.

PR Metadata Assessment

• ❓ QUESTION: The title still carries the PM-XXXXX placeholder instead of a ticket ID; the tracking section links Confluence and [PM-43815] feat: Add dead letter retention for event integrations #8364 but no Jira task.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.64%. Comparing base (dd827fd) to head (3062c71).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8399      +/-   ##
==========================================
+ Coverage   64.61%   64.64%   +0.02%     
==========================================
  Files        2482     2484       +2     
  Lines      106349   106419      +70     
  Branches     9659     9663       +4     
==========================================
+ Hits        68720    68797      +77     
+ Misses      35285    35277       -8     
- Partials     2344     2345       +1     

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

🚀 New features to boost your workflow:

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