Skip to content

Commit

Permalink
fix(error): display 500 server error when the api is not healthy (DEV…
Browse files Browse the repository at this point in the history
…-475) (#673)

* chore(config): useful developer release name

* style(dialog): no border or transition in full-size

* style(dialog): no border or transition in full-size

* fix(login): not sure if this is helpful to avoid errors in rollbar

* chore(error): improve error handler

* refactor: clean up code and imports

* chore(header): get api health status more often

* refactor: change comment

* test(error): fix failing test

* refactor(error): clean up code

* test(error): test error handler

* chore(deps): bump js-lib to latest

* refactor(deps): get correct js-lib version

* test(error): exclude one test

* test(error): bring back the test
  • Loading branch information
kilchenmann committed Mar 18, 2022
1 parent fde5d99 commit b374a3b
Show file tree
Hide file tree
Showing 17 changed files with 308 additions and 39 deletions.
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -33,7 +33,7 @@
"@angular/platform-browser-dynamic": "^13.2.6",
"@angular/router": "^13.2.6",
"@ckeditor/ckeditor5-angular": "^2.0.2",
"@dasch-swiss/dsp-js": "^6.2.2",
"@dasch-swiss/dsp-js": "^7.0.0",
"@datadog/browser-rum": "^3.11.0",
"@ngx-translate/core": "^13.0.0",
"@ngx-translate/http-loader": "6.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src/app/main/action/login-form/login-form.component.html
Expand Up @@ -40,7 +40,7 @@
(click)="login()"
type="submit"
class="full-width submit-button"
[disabled]="!form.valid"
[disabled]="!form?.valid"
[class.mat-primary]="!isError"
[class.mat-warn]="isError">
<app-progress-indicator [color]="'white'" [status]="0" *ngIf="loading" class="login-progress"></app-progress-indicator>
Expand Down
5 changes: 2 additions & 3 deletions src/app/main/action/login-form/login-form.component.ts
@@ -1,8 +1,7 @@
import { AfterViewInit, Component, ElementRef, EventEmitter, Inject, Input, OnInit, Output, ViewChild } from '@angular/core';
import { AfterViewInit, Component, EventEmitter, Inject, Input, OnInit, Output } from '@angular/core';
import { FormBuilder, FormGroup, Validators } from '@angular/forms';
import { ActivatedRoute, Router } from '@angular/router';
import { ApiResponseData, ApiResponseError, KnoraApiConnection, LoginResponse, LogoutResponse } from '@dasch-swiss/dsp-js';
import { CacheService } from '../../cache/cache.service';
import { ApiResponseData, ApiResponseError, KnoraApiConnection, LoginResponse } from '@dasch-swiss/dsp-js';
import { DspApiConnectionToken } from '../../declarations/dsp-api-tokens';
import { ErrorHandlerService } from '../../error/error-handler.service';
import { AuthenticationService } from '../../services/authentication.service';
Expand Down
2 changes: 1 addition & 1 deletion src/app/main/dialog/dialog.component.html
Expand Up @@ -408,7 +408,7 @@

<!-- general error message -->
<div *ngSwitchCase="'error'">
<app-error [status]="data.id"></app-error>
<app-error [status]="data.id" [comment]="data.comment"></app-error>
</div>

<!-- Unknown method (or not yet defined) -->
Expand Down
12 changes: 9 additions & 3 deletions src/app/main/dialog/dialog.component.ts
@@ -1,5 +1,5 @@
import { Component, Inject, OnInit, ViewChild } from '@angular/core';
import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog';
import { Component, Inject, OnInit } from '@angular/core';
import { MatDialogRef, MAT_DIALOG_DATA } from '@angular/material/dialog';
import { ReadResource } from '@dasch-swiss/dsp-js';
import { PropertyInfoObject } from 'src/app/project/ontology/default-data/default-properties';
import { FilteredResources } from 'src/app/workspace/results/list-view/list-view.component';
Expand All @@ -21,6 +21,7 @@ export interface DialogData {
projectCode?: string;
selectedResources?: FilteredResources;
resourceClassDefinition?: string;
fullSize?: boolean;
}

export interface ConfirmationWithComment {
Expand All @@ -42,7 +43,12 @@ export class DialogComponent implements OnInit {
constructor(
public dialogRef: MatDialogRef<DialogComponent>,
@Inject(MAT_DIALOG_DATA) public data: DialogData
) { }
) {
if (this.data.fullSize) {
// do not animate the dialog box
this.dialogRef.addPanelClass('full-size-dialog');
}
}

ngOnInit() { }

Expand Down
53 changes: 52 additions & 1 deletion src/app/main/error/error-handler.service.spec.ts
@@ -1,24 +1,37 @@
import { OverlayContainer } from '@angular/cdk/overlay';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { TestBed } from '@angular/core/testing';
import { MatDialogModule } from '@angular/material/dialog';
import { MatDialog, MatDialogModule, MatDialogRef } from '@angular/material/dialog';
import { MatSnackBarModule } from '@angular/material/snack-bar';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
import { HealthEndpointSystem, MockHealth } from '@dasch-swiss/dsp-js';
import { of } from 'rxjs';
import { DspApiConnectionToken } from '../declarations/dsp-api-tokens';
import { DialogComponent } from '../dialog/dialog.component';
import { ErrorHandlerService } from './error-handler.service';

describe('ErrorHandlerService', () => {
let httpTestingController: HttpTestingController;
let service: ErrorHandlerService;
let overlayContainer: OverlayContainer;

let dialog: MatDialog;

beforeEach(() => {

const apiEndpointSpyObj = {
v2: {
auth: jasmine.createSpyObj('auth', ['logout'])
},
system: {
healthEndpoint: jasmine.createSpyObj('healthEndpoint', ['getHealthStatus'])
}
};

TestBed.configureTestingModule({
imports: [
BrowserAnimationsModule,
HttpClientTestingModule,
MatDialogModule,
MatSnackBarModule
],
Expand All @@ -27,12 +40,50 @@ describe('ErrorHandlerService', () => {
provide: DspApiConnectionToken,
useValue: apiEndpointSpyObj
},
{
provide: MatDialog,
useValue: { open: () => of({ id: 1 }) }
}
]
});
service = TestBed.inject(ErrorHandlerService);

httpTestingController = TestBed.inject(HttpTestingController);

const dspConnSpy = TestBed.inject(DspApiConnectionToken);
(dspConnSpy.system.healthEndpoint as jasmine.SpyObj<HealthEndpointSystem>).getHealthStatus.and.callFake(
() => {
const health = MockHealth.mockStopped();
return of(health);
}
);

overlayContainer = TestBed.inject(OverlayContainer);

dialog = TestBed.inject(MatDialog);
});

afterEach(() => {
httpTestingController.verify();
});

afterEach(async () => {
// angular won't call this for us so we need to do it ourselves to avoid leaks.
overlayContainer.ngOnDestroy();
});

it('should be created', () => {
expect(service).toBeTruthy();
});

it('api is not healthy: should return 503 server error', () => {

const error = require('../../../assets/test-data/api-error-0.json');

service.showMessage(error);

spyOn(dialog, 'open').and.returnValue({ afterClosed: () => of({ id: 1 }) } as MatDialogRef<typeof DialogComponent>);
expect(dialog).toBeDefined();

});
});
31 changes: 27 additions & 4 deletions src/app/main/error/error-handler.service.ts
@@ -1,6 +1,6 @@
import { Inject, Injectable } from '@angular/core';
import { MatDialog, MatDialogConfig } from '@angular/material/dialog';
import { ApiResponseData, ApiResponseError, KnoraApiConnection, LogoutResponse } from '@dasch-swiss/dsp-js';
import { ApiResponseData, ApiResponseError, HealthResponse, KnoraApiConnection, LogoutResponse } from '@dasch-swiss/dsp-js';
import { StatusMsg } from 'src/assets/http/statusMsg';
import { DspApiConnectionToken } from '../declarations/dsp-api-tokens';
import { DialogComponent } from '../dialog/dialog.component';
Expand All @@ -25,9 +25,33 @@ export class ErrorHandlerService {
// in case of (internal) server error
const apiServerError = (error.error && !error.error['response']);

const apiResponseMessage = (error.error['response'] ? error.error['response'].error : undefined);

if ((error.status > 499 && error.status < 600) || apiServerError) {

const status = (apiServerError ? 503 : error.status);
let status = (apiServerError ? 503 : error.status);

// check if the api is healthy:
this._dspApiConnection.system.healthEndpoint.getHealthStatus().subscribe(
(response: ApiResponseData<HealthResponse>) => {
if (response.body.status === 'unhealthy') {
const healthError: ApiResponseError = {
error: response.body.message,
method: response.method,
status: 500,
url: error.url
};
status = 500;
error = healthError;
throw new Error(`ERROR ${status}: Server side error — dsp-api is not healthy`);
} else {
throw new Error(`ERROR ${status}: Server side error — dsp-api not responding`);
}
},
(healthError: ApiResponseError) => {
error = healthError;
}
);

// open error message in full size view
const dialogConfig: MatDialogConfig = {
Expand All @@ -38,7 +62,7 @@ export class ErrorHandlerService {
position: {
top: '0'
},
data: { mode: 'error', id: status },
data: { mode: 'error', id: status, comment: apiResponseMessage, fullSize: true },
disableClose: true
};

Expand All @@ -47,7 +71,6 @@ export class ErrorHandlerService {
dialogConfig
);

throw new Error(`ERROR ${status}: Server side error — dsp-api not responding`);

} else if (error.status === 401 && typeof(error.error) !== 'string') {
// logout if error status is a 401 error and comes from a DSP-JS request
Expand Down
9 changes: 6 additions & 3 deletions src/app/main/error/error.component.html
@@ -1,24 +1,27 @@
<div class="error-page">

<!-- error 403: forbidden -->
<div class="container">
<div class="container" *ngIf="!refresh; else isLoading">
<div class="image">
<img class="error-image" [src]="'/assets/images/' + errorMessage?.image" />
</div>
<div class="text error-message">
<h2 class="mat-title">ERROR {{status}}</h2>
<h1 class="mat-headline">{{errorMessage?.message}}</h1>
<p [innerHTML]="errorMessage.description"></p>
<p *ngIf="comment"><strong>API response:</strong><br>&rarr; {{comment}}</p>
<div [ngSwitch]="errorMessage?.action" class="action">
<button *ngSwitchCase="'goback'" mat-button routerLink="/">
<mat-icon>keyboard_backspace</mat-icon> Please go back to the start page.
</button>
<p *ngSwitchCase="'reload'">
Please come back in a few minutes and try to <a (click)="reload()">reload the page</a>
Please come back in a few minutes and try to <a (click)="reload()">reload the page</a>.
</p>
</div>

</div>
</div>
<ng-template #isLoading>
<app-progress-indicator></app-progress-indicator>
</ng-template>

</div>
20 changes: 19 additions & 1 deletion src/app/main/error/error.component.spec.ts
@@ -1,18 +1,36 @@
import { waitForAsync, ComponentFixture, TestBed } from '@angular/core/testing';
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { MatDialogRef } from '@angular/material/dialog';
import { MatIconModule } from '@angular/material/icon';
import { RouterTestingModule } from '@angular/router/testing';
import { DspApiConnectionToken } from '../declarations/dsp-api-tokens';
import { ErrorComponent } from './error.component';

describe('ErrorComponent', () => {
let component: ErrorComponent;
let fixture: ComponentFixture<ErrorComponent>;

const systemEndpointSpyObj = {
health: {
healthEndpoint: jasmine.createSpyObj('healthEndpoint', ['getHealthStatus'])
}
};

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
declarations: [ErrorComponent],
imports: [
MatIconModule,
RouterTestingModule
],
providers: [
{
provide: MatDialogRef,
useValue: {}
},
{
provide: DspApiConnectionToken,
useValue: systemEndpointSpyObj
},
]
})
.compileComponents();
Expand Down
24 changes: 22 additions & 2 deletions src/app/main/error/error.component.ts
@@ -1,6 +1,8 @@
import { Component, Input, OnInit } from '@angular/core';
import { Component, Inject, Input, OnInit } from '@angular/core';
import { Title } from '@angular/platform-browser';
import { ActivatedRoute } from '@angular/router';
import { ApiResponseData, ApiResponseError, HealthResponse, KnoraApiConnection } from '@dasch-swiss/dsp-js';
import { DspApiConnectionToken } from '../declarations/dsp-api-tokens';

export interface ErrorMsg {
status: number;
Expand All @@ -19,6 +21,10 @@ export class ErrorComponent implements OnInit {

@Input() status: number;

@Input() comment?: string;

refresh = false;

// default error messages
errorMessages: ErrorMsg[] = [
{
Expand Down Expand Up @@ -67,6 +73,7 @@ export class ErrorComponent implements OnInit {
errorMessage: ErrorMsg;

constructor(
@Inject(DspApiConnectionToken) private _dspApiConnection: KnoraApiConnection,
private _titleService: Title,
private _route: ActivatedRoute
) { }
Expand Down Expand Up @@ -99,7 +106,20 @@ export class ErrorComponent implements OnInit {
}

reload() {
window.location.reload();
// get api health status first and reload page only, if the api is running and status is healthy
this.refresh = true;

this._dspApiConnection.system.healthEndpoint.getHealthStatus().subscribe(
(response: ApiResponseData<HealthResponse>) => {
if (response.body.status === 'healthy') {
window.location.reload();
}
},
(error: ApiResponseError) => {
this. refresh = false;
}
);

}

}

0 comments on commit b374a3b

Please sign in to comment.