feat: migrate reporting service contract from logging to reporting module#1221
feat: migrate reporting service contract from logging to reporting module#1221alexs-mparticle wants to merge 5 commits intodevelopmentfrom
Conversation
…dule - Define IErrorReportingService, ILoggingService, ISDKError, ISDKLogEntry contracts - Add ErrorReportingDispatcher and LoggingDispatcher for multi-service fan-out - Decouple Logger from reporting (console-only now) - Remove concrete ReportingLogger implementation (moves to kit) - Remove Rokt-specific URLs from constants - Rename src/logging/ to src/reporting/ - Expose registerErrorReportingService/registerLoggingService on mParticle API
PR SummaryMedium Risk Overview Introduces Updates error handling in Written by Cursor Bugbot for commit 4adf5fa. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| public report(error: ISDKError): void { | ||
| this.services.forEach(s => s.report(error)); | ||
| } |
There was a problem hiding this comment.
Dispatcher fan-out lacks error isolation between services
Medium Severity
ErrorReportingDispatcher.report() and LoggingDispatcher.log() iterate over registered services via forEach without any try-catch. If any externally registered service throws, subsequent services are skipped and the exception propagates to the caller. In identityApiClient.ts, report() is called inside a catch block before critical operations like processQueueOnIdentityFailure and invokeCallback — a throwing service would cause those to be silently skipped. The removed ReportingLogger had internal try-catch preventing this.
Additional Locations (2)
- Add optional chaining for _ErrorReportingDispatcher in identityApiClient so tests without a mock dispatcher don't crash - Add registerErrorReportingService and registerLoggingService to the expected public API keys in instance manager test
This reverts commit e573c1f.
|
|
Duplicate of #1224 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
|
|
||
| this.registerLoggingService = function(service: ILoggingService) { | ||
| self._LoggingDispatcher.register(service); | ||
| }; |
There was a problem hiding this comment.
Register methods crash when called before init
High Severity
registerErrorReportingService and registerLoggingService access self._ErrorReportingDispatcher and self._LoggingDispatcher directly without null guards, but these dispatchers are only created inside runPreConfigFetchInitialization (called during init()). Calling either registration method before init() throws a TypeError. Other public API methods in this file (e.g., setAppVersion, _setWrapperSDKInfo) use queueIfNotInitialized to handle this scenario, but the new methods do not.
Additional Locations (1)
1 participant
Background
What Has Changed
Screenshots/Video
Checklist
Additional Notes
Reference Issue (For employees only. Ignore if you are an outside contributor)