From 88db808b3aa75cf0521a36239b5b1773af314ca2 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Mon, 15 Sep 2025 10:30:05 -0500 Subject: [PATCH 1/3] chore(eslint): added updates to eslint for the pipeline --- src/app/app.config.ts | 27 +++++----- src/app/core/factory/sentry.factory.spec.ts | 19 +++++++ src/app/core/factory/sentry.factory.ts | 49 +++++++++++++++++++ src/app/core/handlers/global-error.handler.ts | 8 --- src/app/core/handlers/index.ts | 1 - .../collections-query-sync.service.ts | 4 +- 6 files changed, 85 insertions(+), 23 deletions(-) create mode 100644 src/app/core/factory/sentry.factory.spec.ts create mode 100644 src/app/core/factory/sentry.factory.ts delete mode 100644 src/app/core/handlers/global-error.handler.ts delete mode 100644 src/app/core/handlers/index.ts diff --git a/src/app/app.config.ts b/src/app/app.config.ts index ac3980d36..47c42c662 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -13,6 +13,7 @@ import { provideRouter, withInMemoryScrolling } from '@angular/router'; import { STATES } from '@core/constants'; import { APPLICATION_INITIALIZATION_PROVIDER } from '@core/factory/application.initialization.factory'; +import { SENTRY_PROVIDER } from '@core/factory/sentry.factory'; import { provideTranslation } from '@core/helpers'; import { authInterceptor, errorInterceptor, viewOnlyInterceptor } from './core/interceptors'; @@ -23,9 +24,15 @@ import * as Sentry from '@sentry/angular'; export const appConfig: ApplicationConfig = { providers: [ - provideZoneChangeDetection({ eventCoalescing: true }), - provideRouter(routes, withInMemoryScrolling({ scrollPositionRestoration: 'top', anchorScrolling: 'enabled' })), - provideStore(STATES, withNgxsReduxDevtoolsPlugin({ disabled: false })), + APPLICATION_INITIALIZATION_PROVIDER, + ConfirmationService, + { + provide: ErrorHandler, + useFactory: () => Sentry.createErrorHandler({ showDialog: false }), + }, + importProvidersFrom(TranslateModule.forRoot(provideTranslation())), + MessageService, + provideAnimations(), providePrimeNG({ theme: { preset: CustomPreset, @@ -38,16 +45,10 @@ export const appConfig: ApplicationConfig = { }, }, }), - provideAnimations(), provideHttpClient(withInterceptors([authInterceptor, viewOnlyInterceptor, errorInterceptor])), - importProvidersFrom(TranslateModule.forRoot(provideTranslation())), - ConfirmationService, - MessageService, - - APPLICATION_INITIALIZATION_PROVIDER, - { - provide: ErrorHandler, - useFactory: () => Sentry.createErrorHandler({ showDialog: false }), - }, + provideRouter(routes, withInMemoryScrolling({ scrollPositionRestoration: 'top', anchorScrolling: 'enabled' })), + provideStore(STATES, withNgxsReduxDevtoolsPlugin({ disabled: false })), + provideZoneChangeDetection({ eventCoalescing: true }), + SENTRY_PROVIDER, ], }; diff --git a/src/app/core/factory/sentry.factory.spec.ts b/src/app/core/factory/sentry.factory.spec.ts new file mode 100644 index 000000000..f3af9aa6f --- /dev/null +++ b/src/app/core/factory/sentry.factory.spec.ts @@ -0,0 +1,19 @@ +import { TestBed } from '@angular/core/testing'; + +import { SENTRY_PROVIDER, SENTRY_TOKEN } from './sentry.factory'; + +import * as Sentry from '@sentry/angular'; + +describe('Factory: Sentry', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [SENTRY_PROVIDER], + }); + }); + + it('should provide the Sentry module via the injection token', () => { + const provided = TestBed.inject(SENTRY_TOKEN); + expect(provided).toBe(Sentry); + expect(typeof provided.captureException).toBe('function'); + }); +}); diff --git a/src/app/core/factory/sentry.factory.ts b/src/app/core/factory/sentry.factory.ts new file mode 100644 index 000000000..baac5068e --- /dev/null +++ b/src/app/core/factory/sentry.factory.ts @@ -0,0 +1,49 @@ +import { InjectionToken } from '@angular/core'; + +import * as Sentry from '@sentry/angular'; + +/** + * Injection token used to provide the Sentry module via Angular's dependency injection system. + * + * This token represents the entire Sentry module (`@sentry/angular`), allowing you to inject + * and use Sentry APIs (e.g., `captureException`, `init`, `setUser`, etc.) in Angular services + * or components. + * + * @example + * ```ts + * const Sentry = inject(SENTRY_TOKEN); + * Sentry.captureException(new Error('Something went wrong')); + * ``` + */ +export const SENTRY_TOKEN = new InjectionToken('Sentry'); + +/** + * Angular provider that binds the `SENTRY_TOKEN` to the actual `@sentry/angular` module. + * + * Use this provider in your module or application configuration to make Sentry injectable. + * + * @example + * ```ts + * providers: [ + * SENTRY_PROVIDER, + * ] + * ``` + * + * Inject the Sentry module via the factory token + * private readonly Sentry = inject(SENTRY_TOKEN); + * + * throwError(): void { + * try { + * throw new Error('Test error for Sentry capture'); + * } catch (error) { + * Send the error to Sentry + * this.Sentry.captureException(error); + * } + * } + * + * @see SENTRY_TOKEN + */ +export const SENTRY_PROVIDER = { + provide: SENTRY_TOKEN, + useValue: Sentry, +}; diff --git a/src/app/core/handlers/global-error.handler.ts b/src/app/core/handlers/global-error.handler.ts deleted file mode 100644 index 6d6bcc907..000000000 --- a/src/app/core/handlers/global-error.handler.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { ErrorHandler, Injectable } from '@angular/core'; - -@Injectable() -export class GlobalErrorHandler implements ErrorHandler { - handleError(error: unknown): void { - console.error('Error:', error); - } -} diff --git a/src/app/core/handlers/index.ts b/src/app/core/handlers/index.ts deleted file mode 100644 index 400695f14..000000000 --- a/src/app/core/handlers/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { GlobalErrorHandler } from './global-error.handler'; diff --git a/src/app/features/collections/services/collections-query-sync.service.ts b/src/app/features/collections/services/collections-query-sync.service.ts index 4aaaecc33..af92b2e50 100644 --- a/src/app/features/collections/services/collections-query-sync.service.ts +++ b/src/app/features/collections/services/collections-query-sync.service.ts @@ -4,6 +4,7 @@ import { effect, inject, Injectable, signal } from '@angular/core'; import { toSignal } from '@angular/core/rxjs-interop'; import { ActivatedRoute, Router } from '@angular/router'; +import { SENTRY_TOKEN } from '@core/factory/sentry.factory'; import { collectionsSortOptions } from '@osf/features/collections/constants'; import { queryParamsKeys } from '@osf/features/collections/constants/query-params-keys.const'; import { CollectionQueryParams } from '@osf/features/collections/models'; @@ -13,6 +14,7 @@ import { SetPageNumber } from '@shared/stores/collections/collections.actions'; @Injectable() export class CollectionsQuerySyncService { + private readonly Sentry = inject(SENTRY_TOKEN); private readonly router = inject(Router); private readonly route = inject(ActivatedRoute); @@ -119,7 +121,7 @@ export class CollectionsQuerySyncService { const parsedFilters: CollectionsFilters = JSON.parse(activeFilters); this.handleParsedFilters(parsedFilters); } catch (error) { - console.error('Error parsing activeFilters from URL:', error); + this.Sentry.captureException(error); } } From 2dc8fa770a1cc74d28e2e49da73e855f30b78701 Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Mon, 15 Sep 2025 10:35:15 -0500 Subject: [PATCH 2/3] chore(linting): fixed some linting --- src/app/core/store/user/user.state.ts | 30 +++++++++---------- .../google-file-picker.component.ts | 9 +++--- src/app/shared/mappers/user/user.mapper.ts | 4 +-- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/app/core/store/user/user.state.ts b/src/app/core/store/user/user.state.ts index da41b3e23..58e2dfd72 100644 --- a/src/app/core/store/user/user.state.ts +++ b/src/app/core/store/user/user.state.ts @@ -268,23 +268,21 @@ export class UserState { }; const apiRequest = UserMapper.toAcceptedTermsOfServiceRequest(updatePayload); - return this.userService - .updateUserAcceptedTermsOfService(currentUser.id, apiRequest) - .pipe( - tap((response: User): void => { - if (response.acceptedTermsOfService) { - ctx.patchState({ - currentUser: { - ...state.currentUser, - data: { - ...currentUser, - acceptedTermsOfService: true, - }, + return this.userService.updateUserAcceptedTermsOfService(currentUser.id, apiRequest).pipe( + tap((response: User): void => { + if (response.acceptedTermsOfService) { + ctx.patchState({ + currentUser: { + ...state.currentUser, + data: { + ...currentUser, + acceptedTermsOfService: true, }, - }); - } - }) - ); + }, + }); + } + }) + ); } @Action(ClearCurrentUser) diff --git a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts index 316b666a1..ed0f2da01 100644 --- a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts +++ b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts @@ -7,6 +7,7 @@ import { Button } from 'primeng/button'; import { ChangeDetectionStrategy, Component, inject, input, OnInit, signal } from '@angular/core'; import { ENVIRONMENT } from '@core/constants/environment.token'; +import { SENTRY_TOKEN } from '@core/factory/sentry.factory'; import { StorageItemModel } from '@osf/shared/models'; import { GoogleFileDataModel } from '@osf/shared/models/files/google-file.data.model'; import { GoogleFilePickerModel } from '@osf/shared/models/files/google-file.picker.model'; @@ -24,6 +25,7 @@ import { GoogleFilePickerDownloadService } from './service/google-file-picker.do changeDetection: ChangeDetectionStrategy.OnPush, }) export class GoogleFilePickerComponent implements OnInit { + private readonly Sentry = inject(SENTRY_TOKEN); readonly #translateService = inject(TranslateService); readonly #googlePicker = inject(GoogleFilePickerDownloadService); readonly #environment = inject(ENVIRONMENT); @@ -68,12 +70,11 @@ export class GoogleFilePickerComponent implements OnInit { this.#initializePicker(); this.#loadOauthToken(); }, - // TODO add this error when the Sentry service is working - //error: (err) => console.error('GAPI modules failed:', err), + error: (err) => + this.Sentry.captureException(err, { level: 'error', tags: { feature: 'google-picker auth' } }), }); }, - // TODO add this error when the Sentry service is working - // error: (err) => console.error('Script load failed:', err), + error: (err) => this.Sentry.captureException(err, { level: 'error', tags: { feature: 'google-picker load' } }), }); } diff --git a/src/app/shared/mappers/user/user.mapper.ts b/src/app/shared/mappers/user/user.mapper.ts index 0389a7873..f26a27793 100644 --- a/src/app/shared/mappers/user/user.mapper.ts +++ b/src/app/shared/mappers/user/user.mapper.ts @@ -1,5 +1,6 @@ import { - User, UserAcceptedTermsOfServiceJsonApi, + User, + UserAcceptedTermsOfServiceJsonApi, UserData, UserDataJsonApi, UserDataResponseJsonApi, @@ -74,5 +75,4 @@ export class UserMapper { accepted_terms_of_service: name.acceptedTermsOfService ?? false, }; } - } From 689f3c1e788ea0078d4105da85bbabf8ffb3cdfc Mon Sep 17 00:00:00 2001 From: Brian Pilati Date: Mon, 15 Sep 2025 11:06:54 -0500 Subject: [PATCH 3/3] chore(error-updates): added hints to the sentry --- .../google-file-picker.component.spec.ts | 63 ++++++++++++++++--- .../google-file-picker.component.ts | 5 +- .../helpers/state-error.handler.spec.ts | 4 +- src/app/shared/helpers/state-error.handler.ts | 7 ++- 4 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.spec.ts b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.spec.ts index 8cd5480b6..ac8db2afc 100644 --- a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.spec.ts +++ b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.spec.ts @@ -1,9 +1,11 @@ import { Store } from '@ngxs/store'; -import { of } from 'rxjs'; +import { Observable, of, throwError } from 'rxjs'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { SENTRY_TOKEN } from '@core/factory/sentry.factory'; + import { GoogleFilePickerDownloadService } from './service/google-file-picker.download.service'; import { GoogleFilePickerComponent } from './google-file-picker.component'; @@ -12,11 +14,21 @@ import { OSFTestingModule, OSFTestingStoreModule } from '@testing/osf.testing.mo describe('Component: Google File Picker', () => { let component: GoogleFilePickerComponent; let fixture: ComponentFixture; + const googlePickerServiceSpy = { - loadScript: jest.fn().mockReturnValue(of(void 0)), - loadGapiModules: jest.fn().mockReturnValue(of(void 0)), + loadScript: jest.fn((): Observable => { + return throwLoadScriptError ? throwError(() => new Error('loadScript failed')) : of(void 0); + }), + loadGapiModules: jest.fn((): Observable => { + return throwLoadGapiError ? throwError(() => new Error('loadGapiModules failed')) : of(void 0); + }), }; + let sentrySpy: any; + + let throwLoadScriptError = false; + let throwLoadGapiError = false; + const handleFolderSelection = jest.fn(); const setDeveloperKey = jest.fn().mockReturnThis(); const setAppId = jest.fn().mockReturnThis(); @@ -40,7 +52,16 @@ describe('Component: Google File Picker', () => { selectSnapshot: jest.fn().mockReturnValue('mock-token'), }; + beforeEach(() => { + throwLoadScriptError = false; + throwLoadGapiError = false; + jest.clearAllMocks(); + }); + beforeAll(() => { + throwLoadScriptError = false; + throwLoadGapiError = false; + window.google = { picker: { Action: null, @@ -54,8 +75,6 @@ describe('Component: Google File Picker', () => { describe('isFolderPicker - true', () => { beforeEach(async () => { - jest.clearAllMocks(); - (window as any).google = { picker: { ViewId: { @@ -86,6 +105,7 @@ describe('Component: Google File Picker', () => { await TestBed.configureTestingModule({ imports: [OSFTestingModule, GoogleFilePickerComponent], providers: [ + { provide: SENTRY_TOKEN, useValue: { captureException: jest.fn() } }, { provide: GoogleFilePickerDownloadService, useValue: googlePickerServiceSpy }, { provide: Store, @@ -94,6 +114,9 @@ describe('Component: Google File Picker', () => { ], }).compileComponents(); + sentrySpy = TestBed.inject(SENTRY_TOKEN); + jest.spyOn(sentrySpy, 'captureException'); + fixture = TestBed.createComponent(GoogleFilePickerComponent); component = fixture.componentInstance; fixture.componentRef.setInput('isFolderPicker', true); @@ -108,6 +131,7 @@ describe('Component: Google File Picker', () => { it('should load script and then GAPI modules and initialize picker', () => { expect(googlePickerServiceSpy.loadScript).toHaveBeenCalled(); expect(googlePickerServiceSpy.loadGapiModules).toHaveBeenCalled(); + expect(sentrySpy.captureException).not.toHaveBeenCalled(); expect(component.visible()).toBeTruthy(); expect(component.isGFPDisabled()).toBeFalsy(); @@ -172,7 +196,6 @@ describe('Component: Google File Picker', () => { describe('isFolderPicker - false', () => { beforeEach(async () => { - jest.clearAllMocks(); (window as any).google = { picker: { ViewId: { @@ -203,6 +226,7 @@ describe('Component: Google File Picker', () => { await TestBed.configureTestingModule({ imports: [OSFTestingStoreModule, GoogleFilePickerComponent], providers: [ + { provide: SENTRY_TOKEN, useValue: { captureException: jest.fn() } }, { provide: GoogleFilePickerDownloadService, useValue: googlePickerServiceSpy }, { provide: Store, @@ -211,6 +235,9 @@ describe('Component: Google File Picker', () => { ], }).compileComponents(); + sentrySpy = TestBed.inject(SENTRY_TOKEN); + jest.spyOn(sentrySpy, 'captureException'); + fixture = TestBed.createComponent(GoogleFilePickerComponent); component = fixture.componentInstance; fixture.componentRef.setInput('isFolderPicker', false); @@ -218,18 +245,38 @@ describe('Component: Google File Picker', () => { itemId: 'root-folder-id', }); fixture.componentRef.setInput('handleFolderSelection', jest.fn()); + }); + + it('should fail to load script', () => { + throwLoadScriptError = true; fixture.detectChanges(); + expect(googlePickerServiceSpy.loadScript).toHaveBeenCalled(); + expect(sentrySpy.captureException).toHaveBeenCalledWith(Error('loadScript failed'), { + tags: { + feature: 'google-picker load', + }, + }); + + expect(component.visible()).toBeFalsy(); + expect(component.isGFPDisabled()).toBeTruthy(); }); - it('should load script and then GAPI modules and initialize picker', () => { + it('should load script and then failr GAPI modules', () => { + throwLoadGapiError = true; + fixture.detectChanges(); expect(googlePickerServiceSpy.loadScript).toHaveBeenCalled(); expect(googlePickerServiceSpy.loadGapiModules).toHaveBeenCalled(); - + expect(sentrySpy.captureException).toHaveBeenCalledWith(Error('loadGapiModules failed'), { + tags: { + feature: 'google-picker auth', + }, + }); expect(component.visible()).toBeFalsy(); expect(component.isGFPDisabled()).toBeTruthy(); }); it('should build the picker with correct configuration', () => { + fixture.detectChanges(); component.createPicker(); expect(window.google.picker.DocsView).toHaveBeenCalledWith('docs'); diff --git a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts index ed0f2da01..80b8e7f87 100644 --- a/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts +++ b/src/app/shared/components/addons/storage-item-selector/google-file-picker/google-file-picker.component.ts @@ -70,11 +70,10 @@ export class GoogleFilePickerComponent implements OnInit { this.#initializePicker(); this.#loadOauthToken(); }, - error: (err) => - this.Sentry.captureException(err, { level: 'error', tags: { feature: 'google-picker auth' } }), + error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker auth' } }), }); }, - error: (err) => this.Sentry.captureException(err, { level: 'error', tags: { feature: 'google-picker load' } }), + error: (err) => this.Sentry.captureException(err, { tags: { feature: 'google-picker load' } }), }); } diff --git a/src/app/shared/helpers/state-error.handler.spec.ts b/src/app/shared/helpers/state-error.handler.spec.ts index ec4a4ba4d..703d3ca90 100644 --- a/src/app/shared/helpers/state-error.handler.spec.ts +++ b/src/app/shared/helpers/state-error.handler.spec.ts @@ -46,7 +46,9 @@ describe('Helper: State Error Handler', () => { }, }); - expect(Sentry.captureException).toHaveBeenCalledWith(error); + expect(Sentry.captureException).toHaveBeenCalledWith(error, { + tags: { feature: 'state error section: mySection', 'state.section': 'mySection' }, + }); await expect(firstValueFrom(result$)).rejects.toThrow('Something went wrong'); }); }); diff --git a/src/app/shared/helpers/state-error.handler.ts b/src/app/shared/helpers/state-error.handler.ts index 6b2fa3dcf..73e60b995 100644 --- a/src/app/shared/helpers/state-error.handler.ts +++ b/src/app/shared/helpers/state-error.handler.ts @@ -6,7 +6,12 @@ import * as Sentry from '@sentry/angular'; export function handleSectionError(ctx: StateContext, section: keyof T, error: Error) { // Report error to Sentry - Sentry.captureException(error); + Sentry.captureException(error, { + tags: { + 'state.section': section.toString(), + feature: `state error section: ${section.toString()}`, + }, + }); // Patch the state to update loading/submitting flags and set the error message ctx.patchState({