diff --git a/src/app/core/auth/auth.actions.ts b/src/app/core/auth/auth.actions.ts index 593d2aceb6..03b6bc1191 100644 --- a/src/app/core/auth/auth.actions.ts +++ b/src/app/core/auth/auth.actions.ts @@ -427,6 +427,15 @@ export class UnsetUserAsIdleAction implements Action { public type: string = AuthActionTypes.UNSET_USER_AS_IDLE; } +/** + * Authentication error actions that include Error payloads. + */ +export type AuthErrorActionsWithErrorPayload + = AuthenticatedErrorAction + | AuthenticationErrorAction + | LogOutErrorAction + | RetrieveAuthenticatedEpersonErrorAction; + /** * Actions type. * @type {AuthActions} @@ -434,9 +443,7 @@ export class UnsetUserAsIdleAction implements Action { export type AuthActions = AuthenticateAction | AuthenticatedAction - | AuthenticatedErrorAction | AuthenticatedSuccessAction - | AuthenticationErrorAction | AuthenticationSuccessAction | CheckAuthenticationTokenAction | CheckAuthenticationTokenCookieAction @@ -453,10 +460,9 @@ export type AuthActions | RetrieveAuthMethodsErrorAction | RetrieveTokenAction | RetrieveAuthenticatedEpersonAction - | RetrieveAuthenticatedEpersonErrorAction | RetrieveAuthenticatedEpersonSuccessAction | SetRedirectUrlAction | RedirectAfterLoginSuccessAction | SetUserAsIdleAction - | UnsetUserAsIdleAction; - + | UnsetUserAsIdleAction + | AuthErrorActionsWithErrorPayload; diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index 0bd25f3649..2919a40fa8 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/src/app/core/auth/auth.effects.ts @@ -1,6 +1,7 @@ import { Injectable, NgZone, + Type, } from '@angular/core'; // import @ngrx import { @@ -50,6 +51,7 @@ import { AuthenticatedSuccessAction, AuthenticationErrorAction, AuthenticationSuccessAction, + AuthErrorActionsWithErrorPayload, CheckAuthenticationTokenCookieAction, LogOutErrorAction, LogOutSuccessAction, @@ -81,6 +83,16 @@ const IDLE_TIMER_IGNORE_TYPES: string[] = [...Object.values(AuthActionTypes).filter((t: string) => t !== AuthActionTypes.UNSET_USER_AS_IDLE), ...Object.values(RequestActionTypes), ...Object.values(NotificationsActionTypes)]; +export function errorToAuthAction$(actionType: Type, error: unknown): Observable { + if (error instanceof Error) { + return observableOf(new actionType(error)); + } + + // If we caught something that's not an Error: complain & drop type safety + console.warn('AuthEffects caught non-Error object:', error); + return observableOf(new actionType(error as any)); +} + @Injectable() export class AuthEffects { @@ -94,7 +106,7 @@ export class AuthEffects { return this.authService.authenticate(action.payload.email, action.payload.password).pipe( take(1), map((response: AuthStatus) => new AuthenticationSuccessAction(response.token)), - catchError((error) => observableOf(new AuthenticationErrorAction(error))), + catchError((error: unknown) => errorToAuthAction$(AuthenticationErrorAction, error)), ); }), )); @@ -109,7 +121,8 @@ export class AuthEffects { switchMap((action: AuthenticatedAction) => { return this.authService.authenticatedUser(action.payload).pipe( map((userHref: string) => new AuthenticatedSuccessAction((userHref !== null), action.payload, userHref)), - catchError((error) => observableOf(new AuthenticatedErrorAction(error)))); + catchError((error: unknown) => errorToAuthAction$(AuthenticatedErrorAction, error)), + ); }), )); @@ -155,7 +168,8 @@ export class AuthEffects { } return user$.pipe( map((user: EPerson) => new RetrieveAuthenticatedEpersonSuccessAction(user.id)), - catchError((error) => observableOf(new RetrieveAuthenticatedEpersonErrorAction(error)))); + catchError((error: unknown) => errorToAuthAction$(RetrieveAuthenticatedEpersonErrorAction, error)), + ); }), )); @@ -163,7 +177,7 @@ export class AuthEffects { switchMap(() => { return this.authService.hasValidAuthenticationToken().pipe( map((token: AuthTokenInfo) => new AuthenticatedAction(token)), - catchError((error) => observableOf(new CheckAuthenticationTokenCookieAction())), + catchError((error: unknown) => observableOf(new CheckAuthenticationTokenCookieAction())), ); }), )); @@ -181,7 +195,7 @@ export class AuthEffects { return new RetrieveAuthMethodsAction(response); } }), - catchError((error) => observableOf(new AuthenticatedErrorAction(error))), + catchError((error: unknown) => errorToAuthAction$(AuthenticatedErrorAction, error)), ); }), )); @@ -192,7 +206,7 @@ export class AuthEffects { return this.authService.refreshAuthenticationToken(null).pipe( take(1), map((token: AuthTokenInfo) => new AuthenticationSuccessAction(token)), - catchError((error) => observableOf(new AuthenticationErrorAction(error))), + catchError((error: unknown) => errorToAuthAction$(AuthenticationErrorAction, error)), ); }), )); @@ -201,7 +215,7 @@ export class AuthEffects { switchMap((action: RefreshTokenAction) => { return this.authService.refreshAuthenticationToken(action.payload).pipe( map((token: AuthTokenInfo) => new RefreshTokenSuccessAction(token)), - catchError((error) => observableOf(new RefreshTokenErrorAction())), + catchError((error: unknown) => observableOf(new RefreshTokenErrorAction())), ); }), )); @@ -245,8 +259,8 @@ export class AuthEffects { switchMap(() => { this.authService.stopImpersonating(); return this.authService.logout().pipe( - map((value) => new LogOutSuccessAction()), - catchError((error) => observableOf(new LogOutErrorAction(error))), + map(() => new LogOutSuccessAction()), + catchError((error: unknown) => errorToAuthAction$(LogOutErrorAction, error)), ); }), )); @@ -272,7 +286,7 @@ export class AuthEffects { return this.authService.retrieveAuthMethodsFromAuthStatus(action.payload) .pipe( map((authMethodModels: AuthMethod[]) => new RetrieveAuthMethodsSuccessAction(authMethodModels)), - catchError((error) => observableOf(new RetrieveAuthMethodsErrorAction())), + catchError(() => observableOf(new RetrieveAuthMethodsErrorAction())), ); }), )); diff --git a/src/app/core/auth/auth.interceptor.ts b/src/app/core/auth/auth.interceptor.ts index bb2f07fad2..f326b614d4 100644 --- a/src/app/core/auth/auth.interceptor.ts +++ b/src/app/core/auth/auth.interceptor.ts @@ -296,7 +296,7 @@ export class AuthInterceptor implements HttpInterceptor { return response; } }), - catchError((error, caught) => { + catchError((error: unknown, caught) => { // Intercept an error response if (error instanceof HttpErrorResponse) { diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index e5ab1c851b..6501f43133 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -58,8 +58,8 @@ export class RequestEffects { return this.restApi.request(request.method, request.href, body, request.options, request.isMultipart).pipe( map((data: RawRestResponse) => this.injector.get(request.getResponseParser()).parse(request, data)), map((response: ParsedResponse) => new RequestSuccessAction(request.uuid, response.statusCode, response.link, response.unCacheableObject)), - catchError((error: RequestError) => { - if (hasValue(error.statusCode)) { + catchError((error: unknown) => { + if (error instanceof RequestError) { // if it's an error returned by the server, complete the request return [new RequestErrorAction(request.uuid, error.statusCode, error.message)]; } else { diff --git a/src/app/core/data/root-data.service.ts b/src/app/core/data/root-data.service.ts index 413a4b4385..b8a0809fc3 100644 --- a/src/app/core/data/root-data.service.ts +++ b/src/app/core/data/root-data.service.ts @@ -42,7 +42,7 @@ export class RootDataService extends BaseDataService { */ checkServerAvailability(): Observable { return this.restService.get(this.halService.getRootHref()).pipe( - catchError((err ) => { + catchError((err: unknown) => { console.error(err); return observableOf(false); }), diff --git a/src/app/core/data/signposting-data.service.ts b/src/app/core/data/signposting-data.service.ts index 5186bd7eb4..d78f75bd11 100644 --- a/src/app/core/data/signposting-data.service.ts +++ b/src/app/core/data/signposting-data.service.ts @@ -39,7 +39,7 @@ export class SignpostingDataService { const baseUrl = `${this.appConfig.rest.baseUrl}`; return this.restService.get(`${baseUrl}/signposting/links/${uuid}`).pipe( - catchError((err) => { + catchError((err: unknown) => { return observableOf([]); }), map((res: RawRestResponse) => res.statusCode === 200 ? res.payload as SignpostingLink[] : []), diff --git a/src/app/core/dspace-rest/dspace-rest.service.spec.ts b/src/app/core/dspace-rest/dspace-rest.service.spec.ts index 334077e91f..4dd856b48e 100644 --- a/src/app/core/dspace-rest/dspace-rest.service.spec.ts +++ b/src/app/core/dspace-rest/dspace-rest.service.spec.ts @@ -1,4 +1,7 @@ -import { HttpHeaders } from '@angular/common/http'; +import { + HttpErrorResponse, + HttpHeaders, +} from '@angular/common/http'; import { HttpClientTestingModule, HttpTestingController, @@ -19,11 +22,14 @@ describe('DspaceRestService', () => { let dspaceRestService: DspaceRestService; let httpMock: HttpTestingController; const url = 'http://www.dspace.org/'; - const mockError: any = { - statusCode: 0, + + const mockError = new HttpErrorResponse({ + status: 0, statusText: 'Unknown Error', - message: 'Http failure response for http://www.dspace.org/: 0 ', - }; + error: { + message: 'Http failure response for http://www.dspace.org/: 0 ', + }, + }); beforeEach(() => { TestBed.configureTestingModule({ @@ -61,24 +67,28 @@ describe('DspaceRestService', () => { req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText }); }); it('should throw an error', () => { - dspaceRestService.get(url).subscribe(() => undefined, (err) => { - expect(err).toEqual(mockError); + dspaceRestService.get(url).subscribe(() => undefined, (err: unknown) => { + expect(err).toEqual(jasmine.objectContaining({ + statusCode: 0, + statusText: 'Unknown Error', + message: 'Http failure response for http://www.dspace.org/: 0 ', + })); }); const req = httpMock.expectOne(url); expect(req.request.method).toBe('GET'); - req.error(mockError); + req.error({ error: mockError } as ErrorEvent); }); it('should log an error', () => { spyOn(console, 'log'); - dspaceRestService.get(url).subscribe(() => undefined, (err) => { + dspaceRestService.get(url).subscribe(() => undefined, (err: unknown) => { expect(console.log).toHaveBeenCalled(); }); const req = httpMock.expectOne(url); expect(req.request.method).toBe('GET'); - req.error(mockError); + req.error({ error: mockError } as ErrorEvent); }); it('when no content-type header is provided, it should use application/json', () => { diff --git a/src/app/core/dspace-rest/dspace-rest.service.ts b/src/app/core/dspace-rest/dspace-rest.service.ts index ac6d155846..bd9da4cf29 100644 --- a/src/app/core/dspace-rest/dspace-rest.service.ts +++ b/src/app/core/dspace-rest/dspace-rest.service.ts @@ -1,8 +1,8 @@ import { HttpClient, + HttpErrorResponse, HttpHeaders, HttpParams, - HttpResponse, } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { @@ -16,9 +16,9 @@ import { import { hasNoValue, - hasValue, isNotEmpty, } from '../../shared/empty.util'; +import { RequestError } from '../data/request-error.model'; import { RestRequestMethod } from '../data/rest-request-method'; import { DSpaceObject } from '../shared/dspace-object.model'; import { RawRestResponse } from './raw-rest-response.model'; @@ -53,24 +53,7 @@ export class DspaceRestService { * An Observable containing the response from the server */ get(absoluteURL: string): Observable { - const requestOptions = { - observe: 'response' as any, - headers: new HttpHeaders({ 'Content-Type': DEFAULT_CONTENT_TYPE }), - }; - return this.http.get(absoluteURL, requestOptions).pipe( - map((res: HttpResponse) => ({ - payload: res.body, - statusCode: res.status, - statusText: res.statusText, - })), - catchError((err) => { - console.log('Error: ', err); - return observableThrowError({ - statusCode: err.status, - statusText: err.statusText, - message: (hasValue(err.error) && isNotEmpty(err.error.message)) ? err.error.message : err.message, - }); - })); + return this.request(RestRequestMethod.GET, absoluteURL); } /** @@ -126,17 +109,23 @@ export class DspaceRestService { statusCode: res.status, statusText: res.statusText, })), - catchError((err) => { - if (hasValue(err.status)) { - return observableThrowError({ - statusCode: err.status, - statusText: err.statusText, - message: (hasValue(err.error) && isNotEmpty(err.error.message)) ? err.error.message : err.message, - }); + catchError((err: unknown) => observableThrowError(() => { + console.log('Error: ', err); + if (err instanceof HttpErrorResponse) { + const error = new RequestError( + (isNotEmpty(err?.error?.message)) ? err.error.message : err.message, + ); + + error.statusCode = err.status; + error.statusText = err.statusText; + + return error; } else { - return observableThrowError(err); + console.error('Cannot construct RequestError from', err); + return err; } - })); + })), + ); } /** diff --git a/src/app/core/services/link-head.service.ts b/src/app/core/services/link-head.service.ts index 29a8dfcbd5..35bed3bb07 100644 --- a/src/app/core/services/link-head.service.ts +++ b/src/app/core/services/link-head.service.ts @@ -47,7 +47,7 @@ export class LinkHeadService { renderer.appendChild(head, link); return renderer; } catch (e) { - console.error('Error within linkService : ', e); + console.error('Error within linkService: ', e); } } @@ -73,7 +73,9 @@ export class LinkHeadService { renderer.removeChild(head, link); } } catch (e) { - console.log('Error while removing tag ' + e.message); + if (e instanceof Error) { + console.error('Error while removing tag: ' + e.message); + } } } } diff --git a/src/app/core/xsrf/xsrf.interceptor.spec.ts b/src/app/core/xsrf/xsrf.interceptor.spec.ts index f95ac11034..1668407e04 100644 --- a/src/app/core/xsrf/xsrf.interceptor.spec.ts +++ b/src/app/core/xsrf/xsrf.interceptor.spec.ts @@ -11,6 +11,7 @@ import { TestBed } from '@angular/core/testing'; import { CookieServiceMock } from '../../shared/mocks/cookie.service.mock'; import { HttpXsrfTokenExtractorMock } from '../../shared/mocks/http-xsrf-token-extractor.mock'; +import { RequestError } from '../data/request-error.model'; import { RestRequestMethod } from '../data/rest-request-method'; import { DspaceRestService } from '../dspace-rest/dspace-rest.service'; import { CookieService } from '../services/cookie.service'; @@ -153,12 +154,13 @@ describe(`XsrfInterceptor`, () => { const mockErrorMessage = 'CSRF token mismatch'; service.request(RestRequestMethod.GET, 'server/api/core/items').subscribe({ - error: (error) => { + error: (error: unknown) => { expect(error).toBeTruthy(); + expect(error instanceof RequestError).toBeTrue(); // ensure mock error (added in below flush() call) is returned. - expect(error.statusCode).toBe(mockErrorCode); - expect(error.statusText).toBe(mockErrorText); + expect((error as RequestError).statusCode).toBe(mockErrorCode); + expect((error as RequestError).statusText).toBe(mockErrorText); // ensure our XSRF-TOKEN cookie exists & has the same value as the new DSPACE-XSRF-TOKEN header expect(cookieService.get('XSRF-TOKEN')).not.toBeNull(); diff --git a/src/app/core/xsrf/xsrf.interceptor.ts b/src/app/core/xsrf/xsrf.interceptor.ts index f3299491e0..91e780590b 100644 --- a/src/app/core/xsrf/xsrf.interceptor.ts +++ b/src/app/core/xsrf/xsrf.interceptor.ts @@ -102,7 +102,7 @@ export class XsrfInterceptor implements HttpInterceptor { } } }), - catchError((error) => { + catchError((error: unknown) => { if (error instanceof HttpErrorResponse) { // For every error that comes back, also check for the custom // DSPACE-XSRF-TOKEN header sent from the backend. diff --git a/src/app/item-page/edit-item-page/item-authorizations/item-authorizations.component.ts b/src/app/item-page/edit-item-page/item-authorizations/item-authorizations.component.ts index c5b33e1b74..ce68524a0a 100644 --- a/src/app/item-page/edit-item-page/item-authorizations/item-authorizations.component.ts +++ b/src/app/item-page/edit-item-page/item-authorizations/item-authorizations.component.ts @@ -180,7 +180,7 @@ export class ItemAuthorizationsComponent implements OnInit, OnDestroy { filter((item: Item) => isNotEmpty(item.bundles)), mergeMap((item: Item) => item.bundles), getFirstSucceededRemoteDataWithNotEmptyPayload(), - catchError((error) => { + catchError((error: unknown) => { console.error(error); return observableOf(buildPaginatedList(null, [])); }), @@ -229,7 +229,7 @@ export class ItemAuthorizationsComponent implements OnInit, OnDestroy { private getBundleBitstreams(bundle: Bundle): Observable> { return bundle.bitstreams.pipe( getFirstSucceededRemoteDataPayload(), - catchError((error) => { + catchError((error: unknown) => { console.error(error); return observableOf(buildPaginatedList(null, [])); }), diff --git a/src/app/shared/subscriptions/subscription-modal/subscription-modal.component.ts b/src/app/shared/subscriptions/subscription-modal/subscription-modal.component.ts index 683633ae2a..0eb17ef835 100644 --- a/src/app/shared/subscriptions/subscription-modal/subscription-modal.component.ts +++ b/src/app/shared/subscriptions/subscription-modal/subscription-modal.component.ts @@ -204,7 +204,7 @@ export class SubscriptionModalComponent implements OnInit { } this.processing$.next(false); }, - error: err => { + error: (err: unknown) => { this.processing$.next(false); }, }); diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 509477ba8b..145854d2bc 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -262,7 +262,7 @@ export class SubmissionObjectEffects { switchMap(([action, state]: [DepositSubmissionAction, any]) => { return this.submissionService.depositSubmission(state.submission.objects[action.payload.submissionId].selfUrl).pipe( map(() => new DepositSubmissionSuccessAction(action.payload.submissionId)), - catchError((error) => observableOf(new DepositSubmissionErrorAction(action.payload.submissionId)))); + catchError((error: unknown) => observableOf(new DepositSubmissionErrorAction(action.payload.submissionId)))); }))); /** diff --git a/src/app/submission/sections/form/section-form.component.ts b/src/app/submission/sections/form/section-form.component.ts index 6b80c325ef..defc78d611 100644 --- a/src/app/submission/sections/form/section-form.component.ts +++ b/src/app/submission/sections/form/section-form.component.ts @@ -341,7 +341,9 @@ export class SubmissionSectionFormComponent extends SectionModelComponent { message: msg, path: '/sections/' + this.sectionData.id, }; - console.error(e.stack); + if (e instanceof Error) { + console.error(e.stack); + } this.sectionService.setSectionError(this.submissionId, this.sectionData.id, sectionError); } } diff --git a/src/app/submission/submission.service.spec.ts b/src/app/submission/submission.service.spec.ts index 1154a76f4b..0ba8e5d9a2 100644 --- a/src/app/submission/submission.service.spec.ts +++ b/src/app/submission/submission.service.spec.ts @@ -29,7 +29,9 @@ import { TestScheduler } from 'rxjs/testing'; import { environment } from '../../environments/environment'; import { storeModuleConfig } from '../app.reducer'; +import { ErrorResponse } from '../core/cache/response.models'; import { RequestService } from '../core/data/request.service'; +import { RequestError } from '../core/data/request-error.model'; import { HttpOptions } from '../core/dspace-rest/dspace-rest.service'; import { RouteService } from '../core/services/route.service'; import { Item } from '../core/shared/item.model'; @@ -959,11 +961,12 @@ describe('SubmissionService test suite', () => { }); it('should catch error from REST endpoint', () => { + const requestError = new RequestError('Internal Server Error'); + requestError.statusCode = 500; + const errorResponse = new ErrorResponse(requestError); + (service as any).restService.getDataById.and.callFake( - () => observableThrowError({ - statusCode: 500, - errorMessage: 'Internal Server Error', - }), + () => observableThrowError(errorResponse), ); service.retrieveSubmission('826').subscribe((r) => { diff --git a/src/app/submission/submission.service.ts b/src/app/submission/submission.service.ts index 1d518eaa3d..312d74bc8a 100644 --- a/src/app/submission/submission.service.ts +++ b/src/app/submission/submission.service.ts @@ -576,8 +576,10 @@ export class SubmissionService { find((submissionObjects: SubmissionObject[]) => isNotUndefined(submissionObjects)), map((submissionObjects: SubmissionObject[]) => createSuccessfulRemoteDataObject( submissionObjects[0])), - catchError((errorResponse: ErrorResponse) => { - return createFailedRemoteDataObject$(errorResponse.errorMessage, errorResponse.statusCode); + catchError((errorResponse: unknown) => { + if (errorResponse instanceof ErrorResponse) { + return createFailedRemoteDataObject$(errorResponse.errorMessage, errorResponse.statusCode); + } }), ); }