Skip to content

Commit

Permalink
Merge pull request #1934 from jhpetersen/enhance_network_error_check
Browse files Browse the repository at this point in the history
fix network error detection due to potential falsy instanceof ProgressEvent evaluation if ProgressEvent is monkey patched by another library
  • Loading branch information
damienbod committed May 9, 2024
2 parents b2e26f7 + 6e82b3f commit 6e3a357
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 14 deletions.
@@ -1,4 +1,4 @@
import { HttpErrorResponse, HttpHeaders } from '@angular/common/http';
import { HttpHeaders } from '@angular/common/http';
import { Injectable, inject } from '@angular/core';
import { Observable, of, throwError, timer } from 'rxjs';
import { catchError, mergeMap, retryWhen, switchMap } from 'rxjs/operators';
Expand All @@ -10,6 +10,7 @@ import { UrlService } from '../../utils/url/url.service';
import { TokenValidationService } from '../../validation/token-validation.service';
import { AuthResult, CallbackContext } from '../callback-context';
import { FlowsDataService } from '../flows-data.service';
import { isNetworkError } from './error-helper';

@Injectable({ providedIn: 'root' })
export class CodeFlowCallbackHandlerService {
Expand Down Expand Up @@ -144,12 +145,7 @@ export class CodeFlowCallbackHandlerService {
return errors.pipe(
mergeMap((error) => {
// retry token refresh if there is no internet connection
if (
error &&
error instanceof HttpErrorResponse &&
error.error instanceof ProgressEvent &&
error.error.type === 'error'
) {
if (isNetworkError(error)) {
const { authority, refreshTokenRetryInSeconds } = config;
const errorMessage = `OidcService code request ${authority} - no internet connection`;

Expand Down
@@ -0,0 +1,57 @@
import { HttpErrorResponse } from '@angular/common/http';
import { isNetworkError } from './error-helper';

describe('error helper', () => {
describe('isNetworkError ', () => {
const HTTP_ERROR = new HttpErrorResponse({});

const CONNECTION_ERROR = new HttpErrorResponse({
error: new ProgressEvent('error'),
status: 0,
statusText: 'Unknown Error',
url: 'https://identity-server.test/openid-connect/token',
});

const UNKNOWN_CONNECTION_ERROR = new HttpErrorResponse({
error: new Error('error'),
status: 0,
statusText: 'Unknown Error',
url: 'https://identity-server.test/openid-connect/token',
});

const PARTIAL_CONNECTION_ERROR = new HttpErrorResponse({
error: new ProgressEvent('error'),
status: 418, // i am a teapot
statusText: 'Unknown Error',
url: 'https://identity-server.test/openid-connect/token',
});

it('returns true on http error with status = 0', () => {
expect(isNetworkError(CONNECTION_ERROR)).toBeTrue();
});

it('returns true on http error with status = 0 and unknown error', () => {
expect(isNetworkError(UNKNOWN_CONNECTION_ERROR)).toBeTrue();
});

it('returns true on http error with status <> 0 and error ProgressEvent', () => {
expect(isNetworkError(PARTIAL_CONNECTION_ERROR)).toBeTrue();
});

it('returns false on non http error', () => {
expect(isNetworkError(new Error('not a HttpErrorResponse'))).toBeFalse();
});

it('returns false on string error', () => {
expect(isNetworkError('not a HttpErrorResponse')).toBeFalse();
});

it('returns false on undefined', () => {
expect(isNetworkError(undefined)).toBeFalse();
});

it('returns false on empty http error', () => {
expect(isNetworkError(HTTP_ERROR)).toBeFalse();
});
});
});
@@ -0,0 +1,14 @@
import { HttpErrorResponse } from '@angular/common/http';

/**
* checks if the error is a network error
* by checking if either internal error is a ProgressEvent with type error
* or another error with status 0
* @param error
* @returns true if the error is a network error
*/
export const isNetworkError = (error: unknown): boolean =>
!!error &&
error instanceof HttpErrorResponse &&
((error.error instanceof ProgressEvent && error.error.type === 'error') ||
(error.status === 0 && !!error.error));
@@ -1,4 +1,4 @@
import { HttpErrorResponse, HttpHeaders } from '@angular/common/http';
import { HttpHeaders } from '@angular/common/http';
import { Injectable, inject } from '@angular/core';
import { Observable, of, throwError, timer } from 'rxjs';
import { catchError, mergeMap, retryWhen, switchMap } from 'rxjs/operators';
Expand All @@ -8,6 +8,7 @@ import { LoggerService } from '../../logging/logger.service';
import { StoragePersistenceService } from '../../storage/storage-persistence.service';
import { UrlService } from '../../utils/url/url.service';
import { AuthResult, CallbackContext } from '../callback-context';
import { isNetworkError } from './error-helper';

@Injectable({ providedIn: 'root' })
export class RefreshTokenCallbackHandlerService {
Expand Down Expand Up @@ -83,12 +84,7 @@ export class RefreshTokenCallbackHandlerService {
return errors.pipe(
mergeMap((error) => {
// retry token refresh if there is no internet connection
if (
error &&
error instanceof HttpErrorResponse &&
error.error instanceof ProgressEvent &&
error.error.type === 'error'
) {
if (isNetworkError(error)) {
const { authority, refreshTokenRetryInSeconds } = config;
const errorMessage = `OidcService code request ${authority} - no internet connection`;

Expand Down

0 comments on commit 6e3a357

Please sign in to comment.