From 34c508073f28636648e54d379ed4ea2634ac6b9b Mon Sep 17 00:00:00 2001 From: Braun Patrik Date: Sun, 9 Jul 2017 14:23:50 +0200 Subject: [PATCH] implementing improved error handling --- backend/middlewares/AdminMWs.ts | 6 +-- backend/middlewares/GalleryMWs.ts | 18 ++------- backend/middlewares/NotificationMWs.ts | 22 +++++++++++ backend/middlewares/RenderingMWs.ts | 7 ++++ backend/middlewares/SharingMWs.ts | 13 ++----- .../thumbnail/ThumbnailGeneratorMWs.ts | 3 +- backend/middlewares/user/AuthenticationMWs.ts | 11 ++---- backend/middlewares/user/UserMWs.ts | 10 ++--- backend/model/NotifocationManager.ts | 28 +++++++++++++ backend/routes/ErrorRouter.ts | 5 +-- backend/routes/LoggerRouter.ts | 3 ++ backend/routes/NotificationRouter.ts | 24 ++++++++++++ backend/server.ts | 16 ++++++++ common/entities/Error.ts | 2 +- common/entities/NotificationDTO.ts | 9 +++++ frontend/app/admin/admin.component.html | 27 +++++++++++-- frontend/app/admin/admin.component.ts | 9 ++++- frontend/app/frame/frame.component.css | 6 +++ frontend/app/frame/frame.component.html | 4 +- frontend/app/frame/frame.component.ts | 4 +- frontend/app/model/notification.service.ts | 39 ++++++++++++++++++- 21 files changed, 213 insertions(+), 53 deletions(-) create mode 100644 backend/middlewares/NotificationMWs.ts create mode 100644 backend/model/NotifocationManager.ts create mode 100644 backend/routes/NotificationRouter.ts create mode 100644 common/entities/NotificationDTO.ts diff --git a/backend/middlewares/AdminMWs.ts b/backend/middlewares/AdminMWs.ts index fe134212..82cd6863 100644 --- a/backend/middlewares/AdminMWs.ts +++ b/backend/middlewares/AdminMWs.ts @@ -37,8 +37,7 @@ export class AdminMWs { return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error saving database settings", err); - return next(new Error(ErrorCodes.SETTINGS_ERROR, err)); + return next(new Error(ErrorCodes.SETTINGS_ERROR, "Error saving database settings", err)); } } @@ -56,8 +55,7 @@ export class AdminMWs { } return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error saving database settings", err); - return next(new Error(ErrorCodes.SETTINGS_ERROR, err)); + return next(new Error(ErrorCodes.SETTINGS_ERROR, "Error saving database settings", err)); } } } diff --git a/backend/middlewares/GalleryMWs.ts b/backend/middlewares/GalleryMWs.ts index 8e5431ef..96ba7577 100644 --- a/backend/middlewares/GalleryMWs.ts +++ b/backend/middlewares/GalleryMWs.ts @@ -8,7 +8,6 @@ import {SearchTypes} from "../../common/entities/AutoCompleteItem"; import {ContentWrapper} from "../../common/entities/ConentWrapper"; import {PhotoDTO} from "../../common/entities/PhotoDTO"; import {ProjectPath} from "../ProjectPath"; -import {Logger} from "../Logger"; import {Config} from "../../common/config/private/Config"; import {UserDTO} from "../../common/entities/UserDTO"; @@ -38,9 +37,7 @@ export class GalleryMWs { return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during listing the directory", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during listing the directory", err)); } } @@ -111,10 +108,7 @@ export class GalleryMWs { req.resultPipe = new ContentWrapper(null, result); return next(); } catch (err) { - - Logger.warn(LOG_TAG, "Error during searching", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during searching", err)); } } @@ -134,9 +128,7 @@ export class GalleryMWs { req.resultPipe = new ContentWrapper(null, result); return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during searching", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during searching", err)); } } @@ -152,9 +144,7 @@ export class GalleryMWs { req.resultPipe = await ObjectManagerRepository.getInstance().SearchManager.autocomplete(req.params.text); return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during searching", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during searching", err)); } } diff --git a/backend/middlewares/NotificationMWs.ts b/backend/middlewares/NotificationMWs.ts new file mode 100644 index 00000000..997e2a91 --- /dev/null +++ b/backend/middlewares/NotificationMWs.ts @@ -0,0 +1,22 @@ +import {NextFunction, Request, Response} from "express"; +import {UserRoles} from "../../common/entities/UserDTO"; +import {NotificationManager} from "../model/NotifocationManager"; + +const LOG_TAG = "[NotificationMWs]"; +export class NotificationMWs { + + + public static list(req: Request, res: Response, next: NextFunction) { + + if (req.session.user.role >= UserRoles.Admin) { + req.resultPipe = NotificationManager.notifications; + } else if (NotificationManager.notifications.length > 0) { + + req.resultPipe = NotificationManager.HasNotification; + } else { + req.resultPipe = []; + } + + return next(); + } +} diff --git a/backend/middlewares/RenderingMWs.ts b/backend/middlewares/RenderingMWs.ts index d81b8f82..a4e6022d 100644 --- a/backend/middlewares/RenderingMWs.ts +++ b/backend/middlewares/RenderingMWs.ts @@ -5,6 +5,8 @@ import {Message} from "../../common/entities/Message"; import {SharingDTO} from "../../common/entities/SharingDTO"; import {Config} from "../../common/config/private/Config"; import {PrivateConfigClass} from "../../common/config/private/PrivateConfigClass"; +import {UserRoles} from "../../common/entities/UserDTO"; +import {NotificationManager} from "../model/NotifocationManager"; export class RenderingMWs { @@ -55,10 +57,15 @@ export class RenderingMWs { public static renderError(err: any, req: Request, res: Response, next: NextFunction): any { + if (err instanceof Error) { + if (!(req.session.user && req.session.user.role >= UserRoles.Developer)) { + delete (err.details); + } let message = new Message(err, null); return res.json(message); } + NotificationManager.error("unknown server error", err); return next(err); } diff --git a/backend/middlewares/SharingMWs.ts b/backend/middlewares/SharingMWs.ts index 5353fc42..25bff569 100644 --- a/backend/middlewares/SharingMWs.ts +++ b/backend/middlewares/SharingMWs.ts @@ -1,7 +1,6 @@ import {NextFunction, Request, Response} from "express"; import {CreateSharingDTO, SharingDTO} from "../../common/entities/SharingDTO"; import {ObjectManagerRepository} from "../model/ObjectManagerRepository"; -import {Logger} from "../Logger"; import {Error, ErrorCodes} from "../../common/entities/Error"; const LOG_TAG = "[SharingMWs]"; @@ -27,9 +26,7 @@ export class SharingMWs { return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during retrieving sharing link", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during retrieving sharing link", err)); } } @@ -71,9 +68,7 @@ export class SharingMWs { return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during creating sharing link", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during creating sharing link", err)); } } @@ -100,9 +95,7 @@ export class SharingMWs { return next(); } catch (err) { - Logger.warn(LOG_TAG, "Error during creating sharing link", err); - console.error(err); - return next(new Error(ErrorCodes.GENERAL_ERROR, err)); + return next(new Error(ErrorCodes.GENERAL_ERROR, "Error during creating sharing link", err)); } } diff --git a/backend/middlewares/thumbnail/ThumbnailGeneratorMWs.ts b/backend/middlewares/thumbnail/ThumbnailGeneratorMWs.ts index f91256de..44cd9307 100644 --- a/backend/middlewares/thumbnail/ThumbnailGeneratorMWs.ts +++ b/backend/middlewares/thumbnail/ThumbnailGeneratorMWs.ts @@ -162,8 +162,7 @@ export class ThumbnailGeneratorMWs { return next(); } } catch (error) { - console.log(error); - return next(new Error(ErrorCodes.THUMBNAIL_GENERATION_ERROR, error)); + return next(new Error(ErrorCodes.THUMBNAIL_GENERATION_ERROR, "Error during generating thumbnail", error)); } } diff --git a/backend/middlewares/user/AuthenticationMWs.ts b/backend/middlewares/user/AuthenticationMWs.ts index 36f024fa..b3291fca 100644 --- a/backend/middlewares/user/AuthenticationMWs.ts +++ b/backend/middlewares/user/AuthenticationMWs.ts @@ -44,8 +44,7 @@ export class AuthenticationMWs { return next(); } } catch (err) { - console.error(err); - return next(new Error(ErrorCodes.CREDENTIAL_NOT_FOUND)); + return next(new Error(ErrorCodes.CREDENTIAL_NOT_FOUND, null, err)); } if (typeof req.session.user === 'undefined') { return next(new Error(ErrorCodes.NOT_AUTHENTICATED)); @@ -109,11 +108,9 @@ export class AuthenticationMWs { return next(); } } catch (err) { - console.error(err); - return next(new Error(ErrorCodes.CREDENTIAL_NOT_FOUND)); + return next(new Error(ErrorCodes.CREDENTIAL_NOT_FOUND, null, err)); } - console.error(err); return next(new Error(ErrorCodes.CREDENTIAL_NOT_FOUND)); } @@ -128,7 +125,7 @@ export class AuthenticationMWs { } //not enough parameter if ((!req.query.sk && !req.params.sharingKey)) { - return next(new Error(ErrorCodes.INPUT_ERROR)); + return next(new Error(ErrorCodes.INPUT_ERROR, "no sharing key provided")); } try { @@ -151,7 +148,7 @@ export class AuthenticationMWs { return next(); } catch (err) { - return next(new Error(ErrorCodes.GENERAL_ERROR)); + return next(new Error(ErrorCodes.GENERAL_ERROR, null, err)); } } diff --git a/backend/middlewares/user/UserMWs.ts b/backend/middlewares/user/UserMWs.ts index 986b7e61..966e5ecf 100644 --- a/backend/middlewares/user/UserMWs.ts +++ b/backend/middlewares/user/UserMWs.ts @@ -22,7 +22,7 @@ export class UserMWs { return next(); } catch (err) { - return next(new Error(ErrorCodes.GENERAL_ERROR)); + return next(new Error(ErrorCodes.GENERAL_ERROR, null, err)); } } @@ -40,7 +40,7 @@ export class UserMWs { return next(); } catch (err) { - return next(new Error(ErrorCodes.USER_CREATION_ERROR)); + return next(new Error(ErrorCodes.USER_CREATION_ERROR, null, err)); } @@ -60,7 +60,7 @@ export class UserMWs { return next(); } catch (err) { - return next(new Error(ErrorCodes.GENERAL_ERROR)); + return next(new Error(ErrorCodes.GENERAL_ERROR, null, err)); } @@ -80,7 +80,7 @@ export class UserMWs { return next(); } catch (err) { - return next(new Error(ErrorCodes.GENERAL_ERROR)); + return next(new Error(ErrorCodes.GENERAL_ERROR, null, err)); } } @@ -99,7 +99,7 @@ export class UserMWs { req.resultPipe = result; next(); } catch (err) { - return next(new Error(ErrorCodes.GENERAL_ERROR)); + return next(new Error(ErrorCodes.GENERAL_ERROR, null, err)); } } diff --git a/backend/model/NotifocationManager.ts b/backend/model/NotifocationManager.ts new file mode 100644 index 00000000..ae5afb52 --- /dev/null +++ b/backend/model/NotifocationManager.ts @@ -0,0 +1,28 @@ +import {NotificationDTO, NotificationType} from "../../common/entities/NotificationDTO"; +export class NotificationManager { + public static notifications: NotificationDTO[] = []; + public static HasNotification: NotificationDTO[] = + [ + { + type: NotificationType.info, + message: "There are unhandled server notification. Login as Administrator to handle them." + } + ]; + + + public static error(message: string, details?: any) { + NotificationManager.notifications.push({ + type: NotificationType.error, + message: message, + details: details + }); + } + + public static warning(message: string, details?: any) { + NotificationManager.notifications.push({ + type: NotificationType.warning, + message: message, + details: details + }); + } +} diff --git a/backend/routes/ErrorRouter.ts b/backend/routes/ErrorRouter.ts index 9a437278..f5c57d35 100644 --- a/backend/routes/ErrorRouter.ts +++ b/backend/routes/ErrorRouter.ts @@ -19,10 +19,9 @@ export class ErrorRouter { private static addGenericHandler(app) { app.use((err: any, req: Request, res: Response, next: Function) => { - //Flush out the stack to the console - Logger.error(err); - next(new Error(ErrorCodes.SERVER_ERROR, "Unknown server side error")); + Logger.error("Unexpected error:", err); + next(new Error(ErrorCodes.SERVER_ERROR, "Unknown server side error", err)); }, RenderingMWs.renderError ); diff --git a/backend/routes/LoggerRouter.ts b/backend/routes/LoggerRouter.ts index 9a9c52ab..0a77c91c 100644 --- a/backend/routes/LoggerRouter.ts +++ b/backend/routes/LoggerRouter.ts @@ -1,6 +1,9 @@ import {NextFunction, Request, Response} from "express"; import {Logger} from "../Logger"; +/** + * Adds logging to express + */ export class LoggerRouter { public static route(app: any) { diff --git a/backend/routes/NotificationRouter.ts b/backend/routes/NotificationRouter.ts new file mode 100644 index 00000000..7481ce71 --- /dev/null +++ b/backend/routes/NotificationRouter.ts @@ -0,0 +1,24 @@ +import {UserRoles} from "../../common/entities/UserDTO"; +import {AuthenticationMWs} from "../middlewares/user/AuthenticationMWs"; +import {RenderingMWs} from "../middlewares/RenderingMWs"; +import {NotificationMWs} from "../middlewares/NotificationMWs"; +import Request = Express.Request; +import Response = Express.Response; + +export class NotificationRouter { + public static route(app: any) { + + this.addGetNotifications(app); + } + + private static addGetNotifications(app) { + app.get("/api/notifications", + AuthenticationMWs.authenticate, + AuthenticationMWs.authorise(UserRoles.Guest), + NotificationMWs.list, + RenderingMWs.renderResult + ); + }; + + +} diff --git a/backend/server.ts b/backend/server.ts index 2c98ee75..d0c206d9 100644 --- a/backend/server.ts +++ b/backend/server.ts @@ -16,6 +16,8 @@ import {LoggerRouter} from "./routes/LoggerRouter"; import {ProjectPath} from "./ProjectPath"; import {ThumbnailGeneratorMWs} from "./middlewares/thumbnail/ThumbnailGeneratorMWs"; import {DiskManager} from "./model/DiskManger"; +import {NotificationRouter} from "./routes/NotificationRouter"; +import {NotificationManager} from "./model/NotifocationManager"; const LOG_TAG = "[server]"; export class Server { @@ -72,6 +74,7 @@ export class Server { GalleryRouter.route(this.app); SharingRouter.route(this.app); AdminRouter.route(this.app); + NotificationRouter.route(this.app); ErrorRouter.route(this.app); @@ -99,6 +102,8 @@ export class Server { } catch (err) { Logger.warn(LOG_TAG, "[MYSQL error]", err); Logger.warn(LOG_TAG, "Error during initializing mysql falling back to memory DB"); + + NotificationManager.warning("Error during initializing mysql falling back to memory DB", err); Config.setDatabaseType(DatabaseType.memory); await ObjectManagerRepository.InitMemoryManagers(); } @@ -112,6 +117,9 @@ export class Server { sharp(); } catch (err) { + NotificationManager.warning("Thumbnail hardware acceleration is not possible." + + " 'Sharp' node module is not found." + + " Falling back to JS based thumbnail generation", err); Logger.warn(LOG_TAG, "[Thumbnail hardware acceleration] sharp module error: ", err); Logger.warn(LOG_TAG, "Thumbnail hardware acceleration is not possible." + " 'Sharp' node module is not found." + @@ -128,6 +136,9 @@ export class Server { if (!err) { return; } + NotificationManager.warning("Thumbnail hardware acceleration is not possible." + + " 'gm' node module is not found." + + " Falling back to JS based thumbnail generation", err); Logger.warn(LOG_TAG, "[Thumbnail hardware acceleration] gm module error: ", err); Logger.warn(LOG_TAG, "Thumbnail hardware acceleration is not possible." + " 'gm' node module is not found." + @@ -136,6 +147,9 @@ export class Server { }); } catch (err) { + NotificationManager.warning("Thumbnail hardware acceleration is not possible." + + " 'gm' node module is not found." + + " Falling back to JS based thumbnail generation", err); Logger.warn(LOG_TAG, "[Thumbnail hardware acceleration] gm module error: ", err); Logger.warn(LOG_TAG, "Thumbnail hardware acceleration is not possible." + " 'gm' node module is not found." + @@ -147,12 +161,14 @@ export class Server { if (Config.Client.Search.searchEnabled == true && ObjectManagerRepository.getInstance().SearchManager.isSupported() == false) { + NotificationManager.warning("Search is not supported with these settings, switching off.."); Logger.warn(LOG_TAG, "Search is not supported with these settings, switching off.."); Config.Client.Search.searchEnabled = false; } if (Config.Client.Sharing.enabled == true && ObjectManagerRepository.getInstance().SharingManager.isSupported() == false) { + NotificationManager.warning("Sharing is not supported with these settings, switching off.."); Logger.warn(LOG_TAG, "Sharing is not supported with these settings, switching off.."); Config.Client.Sharing.enabled = false; } diff --git a/common/entities/Error.ts b/common/entities/Error.ts index 3df5ce3e..de194a5e 100644 --- a/common/entities/Error.ts +++ b/common/entities/Error.ts @@ -21,6 +21,6 @@ export enum ErrorCodes{ } export class Error { - constructor(public code: ErrorCodes, public message?: string) { + constructor(public code: ErrorCodes, public message?: string, public details?: any) { } } diff --git a/common/entities/NotificationDTO.ts b/common/entities/NotificationDTO.ts new file mode 100644 index 00000000..e351f6bf --- /dev/null +++ b/common/entities/NotificationDTO.ts @@ -0,0 +1,9 @@ +export enum NotificationType{ + error, warning, info +} + +export interface NotificationDTO { + type: NotificationType; + message: string; + details?: any; +} diff --git a/frontend/app/admin/admin.component.html b/frontend/app/admin/admin.component.html index 201829bf..a9475d1f 100644 --- a/frontend/app/admin/admin.component.html +++ b/frontend/app/admin/admin.component.html @@ -1,6 +1,27 @@ -
- - +
+ +
+
+

Server notifications

+
+
+ + + + +
+ +
+ To dismiss these notifications, restart the server. +
+ + + +
diff --git a/frontend/app/admin/admin.component.ts b/frontend/app/admin/admin.component.ts index b539c7bc..b7dc4e58 100644 --- a/frontend/app/admin/admin.component.ts +++ b/frontend/app/admin/admin.component.ts @@ -3,6 +3,8 @@ import {AuthenticationService} from "../model/network/authentication.service"; import {Router} from "@angular/router"; import {UserRoles} from "../../../common/entities/UserDTO"; import {Config} from "../../../common/config/public/Config"; +import {NotificationService} from "../model/notification.service"; +import {NotificationType} from "../../../common/entities/NotificationDTO"; @Component({ selector: 'admin', templateUrl: './admin.component.html', @@ -11,8 +13,13 @@ import {Config} from "../../../common/config/public/Config"; export class AdminComponent implements OnInit { userManagementEnable: boolean = false; - constructor(private _authService: AuthenticationService, private _router: Router) { + NotificationType: any; + + constructor(private _authService: AuthenticationService, + private _router: Router, + public notificationService: NotificationService) { this.userManagementEnable = Config.Client.authenticationRequired; + this.NotificationType = NotificationType; } ngOnInit() { diff --git a/frontend/app/frame/frame.component.css b/frontend/app/frame/frame.component.css index 9874daa7..8b081d71 100644 --- a/frontend/app/frame/frame.component.css +++ b/frontend/app/frame/frame.component.css @@ -38,3 +38,9 @@ ng2-slim-loading-bar { display: none; } } + +.badge { + margin-left: -5px; + padding: 0; + background-color: transparent; +} diff --git a/frontend/app/frame/frame.component.html b/frontend/app/frame/frame.component.html index 52dcb90d..c50e0515 100644 --- a/frontend/app/frame/frame.component.html +++ b/frontend/app/frame/frame.component.html @@ -29,7 +29,9 @@
  • + [routerLink]="['/admin']"> + + {{notificationService.notifications.length}}
  • diff --git a/frontend/app/frame/frame.component.ts b/frontend/app/frame/frame.component.ts index a813b85f..2475fc85 100644 --- a/frontend/app/frame/frame.component.ts +++ b/frontend/app/frame/frame.component.ts @@ -4,6 +4,7 @@ import {AuthenticationService} from "../model/network/authentication.service"; import {UserDTO, UserRoles} from "../../../common/entities/UserDTO"; import {Config} from "../../../common/config/public/Config"; import {BehaviorSubject} from "rxjs/BehaviorSubject"; +import {NotificationService} from "../model/notification.service"; @Component({ selector: 'app-frame', @@ -19,7 +20,8 @@ export class FrameComponent { public title: string; isIn: boolean = false; - constructor(private _authService: AuthenticationService) { + constructor(private _authService: AuthenticationService, + public notificationService: NotificationService) { this.user = this._authService.user; this.authenticationRequired = Config.Client.authenticationRequired; this.title = Config.Client.applicationTitle; diff --git a/frontend/app/model/notification.service.ts b/frontend/app/model/notification.service.ts index b02d28a5..191be9b7 100644 --- a/frontend/app/model/notification.service.ts +++ b/frontend/app/model/notification.service.ts @@ -1,5 +1,9 @@ import {Injectable, ViewContainerRef} from "@angular/core"; import {ToastsManager} from "ng2-toastr/ng2-toastr"; +import {NetworkService} from "./network/network.service"; +import {AuthenticationService} from "./network/authentication.service"; +import {NotificationDTO, NotificationType} from "../../../common/entities/NotificationDTO"; +import {UserDTO} from "../../../common/entities/UserDTO"; @Injectable() export class NotificationService { @@ -8,9 +12,42 @@ export class NotificationService { positionClass: "toast-top-center", animate: "flyLeft" }; + notifications: NotificationDTO[] = []; + lastUser: UserDTO = null; - constructor(private _toastr: ToastsManager) { + constructor(private _toastr: ToastsManager, + private _networkService: NetworkService, + private _authService: AuthenticationService) { + this._authService.user.subscribe(() => { + if (this._authService.isAuthenticated() && + (!this.lastUser || + this.lastUser.id != this._authService.user.value.id)) { + this.getServerNotifications(); + } + this.lastUser = this._authService.user.value; + }); + } + + async getServerNotifications() { + this.notifications = await this._networkService.getJson("/notifications"); + this.notifications.forEach((noti) => { + let msg = noti.message; + if (noti.details) { + msg += " Details: " + JSON.stringify(noti.details); + } + switch (noti.type) { + case NotificationType.error: + this.error(msg, "Server error"); + break; + case NotificationType.warning: + this.warning(msg, "Server error"); + break; + case NotificationType.info: + this.info(msg, "Server info"); + break; + } + }) } setRootViewContainerRef(vcr: ViewContainerRef) {