ESLint: fix rxjs/no-implicit-any-catch

In most cases we can deal with the untyped errors by introducing an explicit instanceof check

DspaceRestService includes an unsafe catch/rethrow → made it explicitly typed as `any`, but it should be revisited at some point

Author: ybnd

src/app/core/auth/auth.actions.ts

@@ -427,16 +427,23 @@ 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}
  */
 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;

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$<T extends AuthErrorActionsWithErrorPayload>(actionType: Type<T>, error: unknown): Observable<T> {
+  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,15 +168,16 @@ 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)),
+      );
     }),
   ));
 
   public checkToken$: Observable<Action> = createEffect(() => this.actions$.pipe(ofType(AuthActionTypes.CHECK_AUTHENTICATION_TOKEN),
     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())),
           );
       }),
     ));

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) {
 

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 {

src/app/core/data/root-data.service.ts

@@ -42,7 +42,7 @@ export class RootDataService extends BaseDataService<Root> {
    */
   checkServerAvailability(): Observable<boolean> {
     return this.restService.get(this.halService.getRootHref()).pipe(
-      catchError((err ) => {
+      catchError((err: unknown) => {
         console.error(err);
         return observableOf(false);
       }),

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[] : []),

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', () => {

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<string> containing the response from the server
    */
   get(absoluteURL: string): Observable<RawRestResponse> {
-    const requestOptions = {
-      observe: 'response' as any,
-      headers: new HttpHeaders({ 'Content-Type': DEFAULT_CONTENT_TYPE }),
-    };
-    return this.http.get(absoluteURL, requestOptions).pipe(
-      map((res: HttpResponse<any>) => ({
-        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;
         }
-      }));
+      })),
+    );
   }
 
   /**

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);
+        }
       }
     }
   }

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();