Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(error): display 500 server error when the api is not healthy (DEV-475) #673

Merged
merged 23 commits into from Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
14fccb2
chore(config): useful developer release name
kilchenmann Feb 21, 2022
4954b5c
style(dialog): no border or transition in full-size
kilchenmann Feb 21, 2022
3e0c511
style(dialog): no border or transition in full-size
kilchenmann Feb 21, 2022
868aba9
fix(login): not sure if this is helpful to avoid errors in rollbar
kilchenmann Feb 21, 2022
2562a13
chore(error): improve error handler
kilchenmann Feb 21, 2022
942cbcc
refactor: clean up code and imports
kilchenmann Feb 21, 2022
0380d5e
chore(header): get api health status more often
kilchenmann Feb 21, 2022
2dfa503
refactor: change comment
kilchenmann Feb 22, 2022
0259a88
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Feb 23, 2022
1ec55e8
test(error): fix failing test
kilchenmann Feb 23, 2022
00aaa82
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Feb 23, 2022
8c1c9a8
refactor(error): clean up code
kilchenmann Mar 10, 2022
facc3e3
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Mar 11, 2022
a4e87cb
test(error): test error handler
kilchenmann Mar 16, 2022
9372a29
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Mar 16, 2022
f22c22d
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Mar 16, 2022
97d3362
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Mar 17, 2022
c51788b
Merge branch 'wip/dev-475-catch-server-error' of https://github.com/d…
kilchenmann Mar 17, 2022
f1f9e43
chore(deps): bump js-lib to latest
kilchenmann Mar 17, 2022
ec8b6bc
Merge branch 'main' into wip/dev-475-catch-server-error
kilchenmann Mar 17, 2022
136e48c
refactor(deps): get correct js-lib version
kilchenmann Mar 17, 2022
d5d7d2c
test(error): exclude one test
kilchenmann Mar 17, 2022
1024f29
test(error): bring back the test
kilchenmann Mar 17, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
}
);

}

}