✨ network error removal #930
Conversation
| @@ -18,3 +20,5 @@ export class Observable<T> { | |||
| this.observers.forEach((observer) => observer(data)) | |||
| } | |||
| } | |||
|
|
|||
| export type ErrorObservable = Observable<RawError> | |||
There was a problem hiding this comment.
I'm not sure this type has its place there. I think, either:
- put it in a new
domain/error/types.ts - put it in
tools/error - replace
ErrorObservablewithObservable<RawError>everywhere
There was a problem hiding this comment.
I would simply use Observable<RawError> because it's already used in some places
There was a problem hiding this comment.
Yep, using Observable is probably easier and still well express the intent.
| if (initConfiguration.forwardErrorsToLogs !== false) { | ||
| trackConsoleError(errorObservable) | ||
| trackRuntimeError(errorObservable) | ||
| trackNetworkError(configuration, errorObservable, configuration.isEnabled('remove-network-errors')) |
There was a problem hiding this comment.
When releasing V3, I don't think we'll remove errors for aborted requests for logs, so this extra parameter may be removed. Nonetheless, we can leave it here until we get confirmation from @hdelaby
There was a problem hiding this comment.
Yes, I think this decision does not impact this PR since we are still in V2. Maybe it can be added to the v3 release plan when @hdelaby will confirm.
Codecov Report
@@ Coverage Diff @@
## main #930 +/- ##
==========================================
+ Coverage 88.99% 89.02% +0.02%
==========================================
Files 80 82 +2
Lines 3863 3864 +1
Branches 865 865
==========================================
+ Hits 3438 3440 +2
+ Misses 425 424 -1
Continue to review full report at Codecov.
|
Motivation
RUM errors are not the best way to trigger attention for network requests with a bad status code. We already collect that information within RUM resources. By stopping the generation of RUM errors for these resources, we simplify the value proposition of RUM errors.
Changes
remove-network-errorsflag is enabledTesting
Unit, Locally
I have gone over the contributing documentation.