From d00859664f50ae177c9c17c89b1293aca337005d Mon Sep 17 00:00:00 2001 From: Oleh Paduchak Date: Wed, 1 Oct 2025 15:46:47 +0300 Subject: [PATCH 1/2] fix(errors): properly suppressed datacite errors --- .../core/interceptors/error.interceptor.ts | 12 +++- .../datacite/datacite.service.spec.ts | 58 ++++++++++--------- .../services/datacite/datacite.service.ts | 38 +++++++----- 3 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/app/core/interceptors/error.interceptor.ts b/src/app/core/interceptors/error.interceptor.ts index bb0d5905e..92a513d66 100644 --- a/src/app/core/interceptors/error.interceptor.ts +++ b/src/app/core/interceptors/error.interceptor.ts @@ -1,25 +1,33 @@ -import { throwError } from 'rxjs'; +import { EMPTY, throwError } from 'rxjs'; import { catchError } from 'rxjs/operators'; -import { HttpErrorResponse, HttpInterceptorFn } from '@angular/common/http'; +import { HttpContextToken, HttpErrorResponse, HttpInterceptorFn } from '@angular/common/http'; import { inject } from '@angular/core'; import { Router } from '@angular/router'; +import { SENTRY_TOKEN } from '@core/provider/sentry.provider'; import { hasViewOnlyParam } from '@osf/shared/helpers'; import { LoaderService, ToastService } from '@osf/shared/services'; import { ERROR_MESSAGES } from '../constants'; import { AuthService } from '../services'; +export const BYPASS_ERROR_INTERCEPTOR = new HttpContextToken(() => false); + export const errorInterceptor: HttpInterceptorFn = (req, next) => { const toastService = inject(ToastService); const loaderService = inject(LoaderService); const router = inject(Router); const authService = inject(AuthService); + const sentry = inject(SENTRY_TOKEN); return next(req).pipe( catchError((error: HttpErrorResponse) => { let errorMessage: string; + if (req.context.get(BYPASS_ERROR_INTERCEPTOR)) { + sentry.captureException(error); + return EMPTY; + } if (error.error instanceof ErrorEvent) { errorMessage = error.error.message; diff --git a/src/app/shared/services/datacite/datacite.service.spec.ts b/src/app/shared/services/datacite/datacite.service.spec.ts index 1a803733e..a2f12b5fd 100644 --- a/src/app/shared/services/datacite/datacite.service.spec.ts +++ b/src/app/shared/services/datacite/datacite.service.spec.ts @@ -46,6 +46,7 @@ function assertSuccess( doi: string, event: DataciteEvent ) { + assertSendBeacon(dataciteTrackerAddress, dataciteTrackerRepoId, doi, event); const req = httpMock.expectOne(dataciteTrackerAddress); expect(req.request.method).toBe('POST'); expect(req.request.body).toEqual({ @@ -58,6 +59,24 @@ function assertSuccess( req.flush({}); } +function assertSendBeacon( + dataciteTrackerAddress: string, + dataciteTrackerRepoId: string, + doi: string, + event: DataciteEvent +) { + expect(navigator.sendBeacon).toBeCalledTimes(1); + expect(navigator.sendBeacon).toHaveBeenCalledWith( + dataciteTrackerAddress, + JSON.stringify({ + n: event, + u: window.location.href, + i: dataciteTrackerRepoId, + p: doi, + }) + ); +} + describe('DataciteService', () => { let service: DataciteService; let sentry: jest.Mocked; @@ -67,10 +86,11 @@ describe('DataciteService', () => { const apiDomainUrl = 'https://osf.io'; const dataciteTrackerRepoId = 'repo-123'; describe('with proper configuration', () => { - sentry = { - captureException: jest.fn(), - }; beforeEach(() => { + Object.defineProperty(navigator, 'sendBeacon', { + configurable: true, + value: jest.fn(() => false), // mock implementation that returns true + }); TestBed.configureTestingModule({ providers: [ DataciteService, @@ -164,33 +184,15 @@ describe('DataciteService', () => { assertSuccess(httpMock, dataciteTrackerAddress, dataciteTrackerRepoId, doi, DataciteEvent.DOWNLOAD); }); - it('should log error to sentry', (done: jest.DoneCallback) => { - const doi = 'qwerty'; - const event = 'view'; - service.logIdentifiableView(buildObservable(doi)).subscribe({ - next: () => {}, - error: () => { - throw new Error('The error should have been caught and suppressed by the service.'); - }, - complete: () => { - expect(sentry.captureException).toHaveBeenCalled(); - - done(); - }, - }); + it('navigator success', () => { + (navigator.sendBeacon as jest.Mock).mockReturnValueOnce(true); - const req = httpMock.expectOne(dataciteTrackerAddress); - expect(req.request.method).toBe('POST'); - expect(req.request.body).toEqual({ - n: event, - u: window.location.href, - i: dataciteTrackerRepoId, - p: doi, - }); - expect(req.request.headers.get('Content-Type')).toBe('application/json'); + const doi = 'qwerty'; + const event = DataciteEvent.VIEW; + service.logIdentifiableView(buildObservable(doi)).subscribe(); - const mockError = new ProgressEvent('Internal Server Error'); - req.error(mockError); + httpMock.expectNone(dataciteTrackerAddress); + assertSendBeacon(dataciteTrackerAddress, dataciteTrackerRepoId, doi, event); }); }); diff --git a/src/app/shared/services/datacite/datacite.service.ts b/src/app/shared/services/datacite/datacite.service.ts index 9d058b4ad..dd8611296 100644 --- a/src/app/shared/services/datacite/datacite.service.ts +++ b/src/app/shared/services/datacite/datacite.service.ts @@ -1,11 +1,10 @@ import { EMPTY, filter, map, Observable, of, switchMap, take } from 'rxjs'; -import { catchError } from 'rxjs/operators'; -import { HttpClient } from '@angular/common/http'; +import { HttpClient, HttpContext } from '@angular/common/http'; import { inject, Injectable } from '@angular/core'; +import { BYPASS_ERROR_INTERCEPTOR } from '@core/interceptors'; import { ENVIRONMENT } from '@core/provider/environment.provider'; -import { SENTRY_TOKEN } from '@core/provider/sentry.provider'; import { Identifier, IdentifiersResponseJsonApi } from '@osf/shared/models'; import { DataciteEvent } from '@osf/shared/models/datacite/datacite-event.enum'; @@ -13,7 +12,6 @@ import { DataciteEvent } from '@osf/shared/models/datacite/datacite-event.enum'; providedIn: 'root', }) export class DataciteService { - private readonly Sentry = inject(SENTRY_TOKEN); private readonly http: HttpClient = inject(HttpClient); private readonly environment = inject(ENVIRONMENT); @@ -91,17 +89,25 @@ export class DataciteService { i: this.dataciteTrackerRepoId, p: doi, }; - const headers = { - 'Content-Type': 'application/json', - }; - return this.http.post(this.dataciteTrackerAddress, payload, { headers }).pipe( - map(() => { - return; - }), - catchError((err) => { - this.Sentry.captureException(err); - return of(); - }) - ); + const success = navigator.sendBeacon(this.dataciteTrackerAddress, JSON.stringify(payload)); + if (success) { + return of(void 0); + } else { + const headers = { + 'Content-Type': 'application/json', + }; + const context = new HttpContext(); + context.set(BYPASS_ERROR_INTERCEPTOR, true); + return this.http + .post(this.dataciteTrackerAddress, payload, { + headers, + context, + }) + .pipe( + map(() => { + return; + }) + ); + } } } From 8067dc16db46224958a491513a78a74ffc212fc2 Mon Sep 17 00:00:00 2001 From: Oleh Paduchak Date: Wed, 1 Oct 2025 16:50:23 +0300 Subject: [PATCH 2/2] fix(datacite-tracker): fixed comments --- src/app/shared/services/datacite/datacite.service.spec.ts | 2 +- src/app/shared/services/datacite/datacite.service.ts | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/app/shared/services/datacite/datacite.service.spec.ts b/src/app/shared/services/datacite/datacite.service.spec.ts index a2f12b5fd..2aff40242 100644 --- a/src/app/shared/services/datacite/datacite.service.spec.ts +++ b/src/app/shared/services/datacite/datacite.service.spec.ts @@ -89,7 +89,7 @@ describe('DataciteService', () => { beforeEach(() => { Object.defineProperty(navigator, 'sendBeacon', { configurable: true, - value: jest.fn(() => false), // mock implementation that returns true + value: jest.fn(() => false), }); TestBed.configureTestingModule({ providers: [ diff --git a/src/app/shared/services/datacite/datacite.service.ts b/src/app/shared/services/datacite/datacite.service.ts index dd8611296..65f164292 100644 --- a/src/app/shared/services/datacite/datacite.service.ts +++ b/src/app/shared/services/datacite/datacite.service.ts @@ -103,11 +103,7 @@ export class DataciteService { headers, context, }) - .pipe( - map(() => { - return; - }) - ); + .pipe(map(() => undefined)); } } }