Skip to content

Commit

Permalink
bugfix: CXSPA-8359 unknown HTTP error printed twice to the console
Browse files Browse the repository at this point in the history
  • Loading branch information
pawelfras committed Sep 9, 2024
1 parent 5ad4961 commit 0a53707
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 6 deletions.
2 changes: 1 addition & 1 deletion projects/core/src/cms/config/default-cms-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const defaultCmsModuleConfig: CmsConfig = {
endpoints: {
component: 'cms/components/${id}',
components: 'cms/components',
pages: 'cms/pages',
pages: 'cms/pages2',
page: 'cms/pages/${id}',
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { HttpErrorResponse } from '@angular/common/http';
import * as AngularCore from '@angular/core';
import { TestBed } from '@angular/core/testing';
import { Priority } from '@spartacus/core';
import { FeatureConfigService, LoggerService, Priority } from '@spartacus/core';
import { GlobalMessageService } from '../../../facade';
import { UnknownErrorHandler } from './unknown-error.handler';

Expand All @@ -16,6 +17,8 @@ describe('UnknownErrorHandler', () => {
provide: GlobalMessageService,
useClass: MockGlobalMessageService,
},
FeatureConfigService,
LoggerService,
],
});
service = TestBed.inject(UnknownErrorHandler);
Expand All @@ -32,4 +35,83 @@ describe('UnknownErrorHandler', () => {
it('should have fallback priority ', () => {
expect(service.getPriority()).toBe(Priority.FALLBACK);
});

//remove together with `ssrStrictErrorHandlingForHttpAndNgrx` feature toggle
describe('with `ssrStrictErrorHandlingForHttpAndNgrx` feature toggle', () => {
let featureConfigService: FeatureConfigService;
let loggerService: LoggerService;

beforeEach(() => {
featureConfigService = TestBed.inject(FeatureConfigService);
loggerService = TestBed.inject(LoggerService);
spyOn(loggerService, 'warn');
});

describe('when the feature is enabled', () => {
beforeEach(() => {
spyOn(featureConfigService, 'isEnabled').and.callFake(
(val: string) => val === 'ssrStrictErrorHandlingForHttpAndNgrx'
);
});
it('should log error in dev mode', () => {
spyOnProperty(AngularCore, 'isDevMode').and.returnValue(() => true);
service.handleError(
{} as any,
{ message: 'error' } as HttpErrorResponse
);
expect(loggerService.warn).toHaveBeenCalledWith(
'An unknown http error occurred\n',
'error'
);
});

it('should not log error if it is not a dev mode', () => {
spyOnProperty(AngularCore, 'isDevMode').and.returnValue(() => false);
service.handleError(
{} as any,
{ message: 'error' } as HttpErrorResponse
);
expect(loggerService.warn).not.toHaveBeenCalled();
});
});

describe('when the feature is disabled', () => {
beforeEach(() => {
spyOn(featureConfigService, 'isEnabled').and.callFake(
(val: string) => val !== 'ssrStrictErrorHandlingForHttpAndNgrx'
);
});
it('should log error in dev mode', () => {
spyOnProperty(AngularCore, 'isDevMode').and.returnValue(() => true);
service.handleError(
{} as any,
{ message: 'error' } as HttpErrorResponse
);
expect(loggerService.warn).toHaveBeenCalledWith(
'An unknown http error occurred\n',
'error'
);
});
it('should log error in SSR', () => {
spyOn(service as any, 'isSsr').and.returnValue(true);
service.handleError(
{} as any,
{ message: 'error' } as HttpErrorResponse
);
expect(loggerService.warn).toHaveBeenCalledWith(
'An unknown http error occurred\n',
'error'
);
});
it('should not log error if it is not a dev mode or it is not Ssr', () => {
spyOnProperty(AngularCore, 'isDevMode').and.returnValue(() => false);
spyOn(service as any, 'isSsr').and.returnValue(false);
service.handleError(
{} as any,
{ message: 'error' } as HttpErrorResponse
);
expect(loggerService.warn).not.toHaveBeenCalled();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { HttpErrorResponse, HttpRequest } from '@angular/common/http';
import { Injectable, inject, isDevMode } from '@angular/core';

import { FeatureConfigService } from '../../../../features-config';
import { LoggerService } from '../../../../logger';
import { Priority } from '../../../../util/applicable';
import { HttpResponseStatus } from '../../../models/response-status.model';
Expand All @@ -20,9 +21,10 @@ import { HttpErrorHandler } from '../http-error.handler';
providedIn: 'root',
})
export class UnknownErrorHandler extends HttpErrorHandler {
responseStatus = HttpResponseStatus.UNKNOWN;

protected logger = inject(LoggerService);
protected featureConfigService = inject(FeatureConfigService);

responseStatus = HttpResponseStatus.UNKNOWN;

/**
* hasMatch always returns true, to mach all errors
Expand All @@ -32,7 +34,16 @@ export class UnknownErrorHandler extends HttpErrorHandler {
}

handleError(_request: HttpRequest<any>, errorResponse: HttpErrorResponse) {
if (isDevMode() || this.isSsr()) {
const shouldLogError = this.featureConfigService.isEnabled(
'ssrStrictErrorHandlingForHttpAndNgrx'
)
? isDevMode()
: isDevMode() || this.isSsr();
console.log('shouldLogError', shouldLogError);

// Error will be handled and logged by the `HttpErrorHandlerInterceptor`.
// After removing the `ssrStrictErrorHandlingForHttpAndNgrx` feature flag, error will be logged only in dev mode.
if (shouldLogError) {
this.logger.warn(
`An unknown http error occurred\n`,
errorResponse.message
Expand All @@ -44,6 +55,6 @@ export class UnknownErrorHandler extends HttpErrorHandler {
* Fallback priority assures that the handler is used as a last resort
*/
getPriority() {
return Priority.FALLBACK;
return Priority.HIGH;
}
}

0 comments on commit 0a53707

Please sign in to comment.