[PM-34788] Fix silent error swallowing in saveRiskInsightsReport$#20180
Open
[PM-34788] Fix silent error swallowing in saveRiskInsightsReport$#20180
Conversation
The catchError block returned EMPTY, silently swallowing encryption and API errors. This caused the UI to hang on "Compiling insights..." with no error feedback and silent data loss. Removing the block lets errors propagate to the orchestrator's existing catchError handler, which produces a proper ReportStatus.Error state. [PM-34788]
Add tests verifying that encryption failures and API save errors propagate instead of being silently swallowed. Add orchestrator test confirming save errors produce ReportStatus.Error state. [PM-34788]
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20180 +/- ##
==========================================
- Coverage 46.99% 46.98% -0.02%
==========================================
Files 3889 3891 +2
Lines 117258 117302 +44
Branches 17936 17934 -2
==========================================
+ Hits 55106 55113 +7
- Misses 59672 59709 +37
Partials 2480 2480 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
Great job! No new security vulnerabilities introduced in this pull request |
The orchestrator's catchError already emitted ReportStatus.Error when
saveRiskInsightsReport$ failed, but the UI remained stuck on
"Compiling insights..." because:
1. The _reportProgressSubject was never reset to null on error, so the
progress bar hung on "Compiling insights..." instead of hiding
2. No component subscribed to ReportStatus.Error to show user feedback
Reset _reportProgressSubject in the orchestrator's catchError to hide
the progress bar instead of hanging on "Compiling insights...", and
subscribe to reportStatus$ in the component to show an error toast
matching the existing DIRT toast pattern.
[PM-34788]
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-34788
📔 Objective
saveRiskInsightsReport$()in the V1 report service caught all errors and returnedEMPTY, silently swallowing encryption and API failures. This caused the UI to hang permanently on "Compiling insights..." with no error feedback and silent data loss of the generated report.Removing the
catchError(() => EMPTY)block inrisk-insights-report.service.tslets errors propagate naturally through the RxJS pipe to the orchestrator's existingcatchErrorhandler inrisk-insights-orchestrator.service.ts, which produces aReportStatus.Errorstate. The innercatchErrorwas intercepting errors first and converting them to a silent completion, so the orchestrator's handler never saw them.The orchestrator's
catchErroralso needed to reset_reportProgressSubjecttonullso the loading component hides instead of hanging on "Compiling insights...", and the component needed to subscribe toReportStatus.Errorto show an error toast.What changed:
catchError(() => EMPTY)block inrisk-insights-report.service.tsand cleaned up the unusedEMPTYimport_reportProgressSubjectin the orchestrator'scatchErrorto hide the progress bar on error (risk-insights-orchestrator.service.ts)risk-insights.component.tsthat subscribes toreportStatus$and shows "An error occurred while generating the report. Try again." onReportStatus.ErrorreportGenerationFailedi18n key inmessages.jsonrisk-insights-report.service.spec.tsReportStatus.Errorinrisk-insights-orchestrator.service.spec.tsThis is a V1-only fix. The V2 code path (
DefaultReportPersistenceService) already has proper error handling. This bug goes away when V1 is removed (PM-31947), but V1 is still the active production path for orgs without the V2 flag enabled.Related:
return EMPTYpattern in the fetch method of the same file📸 Screenshots
Before
Screen.Recording.2026-04-15.at.6.11.43.PM.mov
After
Screen.Recording.2026-04-15.at.6.13.03.PM.mov