From acb470f0cc35279b11f3e0a68fff87e7505cc475 Mon Sep 17 00:00:00 2001 From: chinlinlee Date: Sat, 11 Nov 2023 01:31:11 +0800 Subject: [PATCH] refactor: add `ApiErrorArrayHandler` to handler response of custom errors --- api/WADO-URI/controller/retrieveInstance.js | 2 +- api/WADO-URI/service/WADO-URI.service.js | 59 ++-------- .../controller/QIDO-RS/base.controller.js | 5 +- .../controller/WADO-RS/base.controller.js | 5 +- .../WADO-RS/bulkdata/base.controller.js | 5 +- .../controller/WADO-RS/bulkdata/bulkdata.js | 5 +- .../WADO-RS/deletion/base.controller.js | 12 +- .../WADO-RS/metadata/base.controller.js | 5 +- .../WADO-RS/rendered/base.controller.js | 5 +- error/api-errors.handler.js | 106 ++++++++++++++++++ error/controller.handler.js | 24 ---- 11 files changed, 135 insertions(+), 98 deletions(-) create mode 100644 error/api-errors.handler.js delete mode 100644 error/controller.handler.js diff --git a/api/WADO-URI/controller/retrieveInstance.js b/api/WADO-URI/controller/retrieveInstance.js index f80342f3..806168e8 100644 --- a/api/WADO-URI/controller/retrieveInstance.js +++ b/api/WADO-URI/controller/retrieveInstance.js @@ -5,8 +5,8 @@ const { ApiLogger } = require("../../../utils/logs/api-logger"); class RetrieveSingleInstanceController extends Controller { constructor(req, res) { super(req, res); - this.service = new WadoUriService(req, res); this.logger = new ApiLogger(this.request, "WADO-URI"); + this.service = new WadoUriService(req, res, this.logger); } async mainProcess() { diff --git a/api/WADO-URI/service/WADO-URI.service.js b/api/WADO-URI/service/WADO-URI.service.js index 77c71940..36365fa5 100644 --- a/api/WADO-URI/service/WADO-URI.service.js +++ b/api/WADO-URI/service/WADO-URI.service.js @@ -12,6 +12,7 @@ const { AuditManager } = require("@models/DICOM/audit/auditManager"); const { EventType } = require("@models/DICOM/audit/eventType"); const { EventOutcomeIndicator } = require("@models/DICOM/audit/auditUtils"); const { DicomWebService } = require("@root/api/dicom-web/service/dicom-web.service"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class WadoUriService { @@ -20,9 +21,10 @@ class WadoUriService { * @param {import("http").IncomingMessage} req * @param {import("http").ServerResponse} res */ - constructor(req, res) { + constructor(req, res, apiLogger) { this.request = req; this.response = res; + this.apiLogger = apiLogger; this.auditBeginTransferring(); } @@ -38,24 +40,8 @@ class WadoUriService { } catch (e) { this.auditInstanceTransferred(EventOutcomeIndicator.MajorFailure); - if (e instanceof NotFoundInstanceError) { - this.response.writeHead(404, { - "Content-Type": "application/dicom+json" - }); - return this.response.end(); - } else if (e instanceof InstanceGoneError) { - this.response.writeHead(410, { - "Content-Type": "application/dicom+json" - }); - return this.response.end(JSON.stringify({ - Details: e.message, - HttpStatus: 410, - Message: "Image Gone", - Method: "GET" - })); - } - - throw e; + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } @@ -72,39 +58,8 @@ class WadoUriService { } catch (e) { this.auditInstanceTransferred(EventOutcomeIndicator.MajorFailure); - if (e instanceof NotFoundInstanceError) { - - this.response.writeHead(404, { - "Content-Type": "application/dicom+json" - }); - return this.response.end(); - - } else if (e instanceof InvalidFrameNumberError) { - - this.response.writeHead(400, { - "Content-Type": "application/dicom+json" - }); - - return this.response.end(JSON.stringify({ - Details: e.message, - HttpStatus: 400, - Message: "Bad request", - Method: "GET" - })); - - } else if (e instanceof InstanceGoneError) { - this.response.writeHead(410, { - "Content-Type": "application/dicom+json" - }); - return this.response.end(JSON.stringify({ - Details: e.message, - HttpStatus: 410, - Message: "Image Gone", - Method: "GET" - })); - } - - throw e; + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } diff --git a/api/dicom-web/controller/QIDO-RS/base.controller.js b/api/dicom-web/controller/QIDO-RS/base.controller.js index d9a2f162..d94d85e1 100644 --- a/api/dicom-web/controller/QIDO-RS/base.controller.js +++ b/api/dicom-web/controller/QIDO-RS/base.controller.js @@ -1,7 +1,7 @@ -const { ControllerErrorHandler } = require("@error/controller.handler"); const { Controller } = require("@root/api/controller.class"); const { ApiLogger } = require("@root/utils/logs/api-logger"); const { QidoRsService } = require("./service/QIDO-RS.service"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseQueryController extends Controller { constructor(req, res) { @@ -22,7 +22,8 @@ class BaseQueryController extends Controller { let qidoRsService = new QidoRsService(this.request, this.response, this.level); await qidoRsService.getAndResponseDicomJson(); } catch (e) { - return ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } } diff --git a/api/dicom-web/controller/WADO-RS/base.controller.js b/api/dicom-web/controller/WADO-RS/base.controller.js index 21658f46..7636a755 100644 --- a/api/dicom-web/controller/WADO-RS/base.controller.js +++ b/api/dicom-web/controller/WADO-RS/base.controller.js @@ -4,7 +4,7 @@ const { EventOutcomeIndicator } = require("@models/DICOM/audit/auditUtils"); const { WADOZip } = require("./service/WADOZip"); const { ApiLogger } = require("@root/utils/logs/api-logger"); const { sendNotSupportedMediaType, getAcceptType, supportInstanceMultipartType, ImageMultipartWriter, InstanceImagePathFactory, multipartContentTypeWriter, StudyImagePathFactory, SeriesImagePathFactory } = require("./service/WADO-RS.service"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseRetrieveController extends Controller { constructor(req, res) { @@ -32,7 +32,8 @@ class BaseRetrieveController extends Controller { return sendNotSupportedMediaType(this.response, this.request.headers.accept); } catch (e) { - return ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } diff --git a/api/dicom-web/controller/WADO-RS/bulkdata/base.controller.js b/api/dicom-web/controller/WADO-RS/bulkdata/base.controller.js index 646ba1cd..7163a655 100644 --- a/api/dicom-web/controller/WADO-RS/bulkdata/base.controller.js +++ b/api/dicom-web/controller/WADO-RS/bulkdata/base.controller.js @@ -2,7 +2,7 @@ const { Controller } = require("@root/api/controller.class"); const { StudyBulkDataFactory, BulkDataService } = require("./service/bulkdata"); const { ApiLogger } = require("@root/utils/logs/api-logger"); const { StudyImagePathFactory } = require("../service/WADO-RS.service"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseBulkDataController extends Controller { constructor(req, res) { @@ -40,7 +40,8 @@ class BaseBulkDataController extends Controller { bulkDataService.multipartWriter.writeFinalBoundary(); return this.response.end(); } catch(e) { - return ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } } diff --git a/api/dicom-web/controller/WADO-RS/bulkdata/bulkdata.js b/api/dicom-web/controller/WADO-RS/bulkdata/bulkdata.js index a199fa29..9b4ba8a1 100644 --- a/api/dicom-web/controller/WADO-RS/bulkdata/bulkdata.js +++ b/api/dicom-web/controller/WADO-RS/bulkdata/bulkdata.js @@ -5,7 +5,7 @@ const { BulkDataService, SpecificBulkDataFactory } = require("./service/bulkdata const { getInternalServerErrorMessage } = require("../../../../../utils/errorResponse/errorResponseMessage"); const { BaseBulkDataController } = require("./base.controller"); const { InstanceImagePathFactory } = require("../service/WADO-RS.service"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BulkDataController extends BaseBulkDataController { constructor(req, res) { @@ -31,7 +31,8 @@ class BulkDataController extends BaseBulkDataController { bulkDataService.multipartWriter.writeFinalBoundary(); return this.response.end(); } catch(e) { - return ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } diff --git a/api/dicom-web/controller/WADO-RS/deletion/base.controller.js b/api/dicom-web/controller/WADO-RS/deletion/base.controller.js index d65ec5d3..5d455f6f 100644 --- a/api/dicom-web/controller/WADO-RS/deletion/base.controller.js +++ b/api/dicom-web/controller/WADO-RS/deletion/base.controller.js @@ -3,7 +3,7 @@ const { ApiLogger } = require("@root/utils/logs/api-logger"); const { DeleteService } = require("./service/delete"); const { NotFoundInstanceError } = require("@error/dicom-instance"); const { getNotFoundErrorMessage, getInternalServerErrorMessage } = require("@root/utils/errorResponse/errorResponseMessage"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseDeleteController extends Controller { constructor(req, res) { @@ -26,14 +26,8 @@ class BaseDeleteController extends Controller { Method: "DELETE" }); } catch(e) { - - if (e instanceof NotFoundInstanceError) { - return this.response.status(404).json( - getNotFoundErrorMessage(e.message) - ); - } - - return ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } diff --git a/api/dicom-web/controller/WADO-RS/metadata/base.controller.js b/api/dicom-web/controller/WADO-RS/metadata/base.controller.js index fcacad84..9c91e8a8 100644 --- a/api/dicom-web/controller/WADO-RS/metadata/base.controller.js +++ b/api/dicom-web/controller/WADO-RS/metadata/base.controller.js @@ -2,7 +2,7 @@ const { Controller } = require("@root/api/controller.class"); const { ApiLogger } = require("@root/utils/logs/api-logger"); const { StudyImagePathFactory } = require("../service/WADO-RS.service"); const { MetadataService } = require("../service/metadata.service"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseRetrieveMetadataController extends Controller { constructor(req, res) { @@ -33,7 +33,8 @@ class BaseRetrieveMetadataController extends Controller { return this.response.end(); } catch (e) { - ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } } diff --git a/api/dicom-web/controller/WADO-RS/rendered/base.controller.js b/api/dicom-web/controller/WADO-RS/rendered/base.controller.js index d0b31825..5594de04 100644 --- a/api/dicom-web/controller/WADO-RS/rendered/base.controller.js +++ b/api/dicom-web/controller/WADO-RS/rendered/base.controller.js @@ -6,7 +6,7 @@ const { const errorResponse = require("../../../../../utils/errorResponse/errorResponseMessage"); const { ApiLogger } = require("../../../../../utils/logs/api-logger"); const { Controller } = require("../../../../controller.class"); -const { ControllerErrorHandler } = require("@error/controller.handler"); +const { ApiErrorArrayHandler } = require("@error/api-errors.handler"); class BaseRetrieveRenderedController extends Controller { /** @@ -61,7 +61,8 @@ class BaseRetrieveRenderedController extends Controller { return this.response.end(); } catch(e) { - ControllerErrorHandler.raiseInternalServerError(e, this.apiLogger, this.response); + let apiErrorArrayHandler = new ApiErrorArrayHandler(this.response, this.apiLogger, e); + return apiErrorArrayHandler.doErrorResponse(); } } } diff --git a/error/api-errors.handler.js b/error/api-errors.handler.js new file mode 100644 index 00000000..eba07b06 --- /dev/null +++ b/error/api-errors.handler.js @@ -0,0 +1,106 @@ +const { getInternalServerErrorMessage, getNotFoundErrorMessage } = require("@root/utils/errorResponse/errorResponseMessage"); +const { ApiLogger } = require("@root/utils/logs/api-logger"); + + +class DicomWebServiceErrorHandler { + + static doErrorResponse(response, e) { + return response.status(e.code).json({ + status: e.status, + message: e.message + }); + } +} + +class NotFoundInstanceErrorHandler { + static doErrorResponse(response, e) { + response.writeHead(404, { + "Content-Type": "application/dicom+json" + }); + return response.end(JSON.stringify(getNotFoundErrorMessage(e.message))); + } +} + +class InstanceGoneErrorHandler { + static doErrorResponse(response, e) { + response.writeHead(410, { + "Content-Type": "application/dicom+json" + }); + return response.end(JSON.stringify({ + Details: e.message, + HttpStatus: 410, + Message: "Image Gone", + Method: "GET" + })); + } +} + +class InvalidFrameNumberErrorHandler { + static doErrorResponse(response, e) { + response.writeHead(400, { + "Content-Type": "application/dicom+json" + }); + + return response.end(JSON.stringify({ + Details: e.message, + HttpStatus: 400, + Message: "Bad request", + Method: "GET" + })); + } +} + +const ApiErrorHandlerMapping = { + "DicomWebServiceError": DicomWebServiceErrorHandler, + "NotFoundInstanceError": NotFoundInstanceErrorHandler, + "InstanceGoneError": InstanceGoneErrorHandler, + "InvalidFrameNumberError": InvalidFrameNumberErrorHandler +}; + +class ApiErrorArrayHandler { + /** + * @param {import("express").Response} res + * @param {ApiLogger} apiLogger + * @param {Error} e + */ + constructor(res, apiLogger, e) { + this.response = res; + /** @type {ApiLogger} */ + this.apiLogger = apiLogger; + /** @type {Error} */ + this.error = e; + } + + doErrorResponse() { + if (this.isCustomError(this.error)) { + this.apiLogger.logger.error(this.error); + return ApiErrorHandlerMapping[this.error.name].doErrorResponse(this.response, this.error); + } else { + ApiErrorArrayHandler.raiseInternalServerError(this.error, this.response, this.apiLogger); + } + } + + /** + * + * @param {import("express").Response} response + * @param {ApiLogger} apiLogger + * @param {Error} e + */ + static raiseInternalServerError(response, apiLogger, e) { + apiLogger.logger.error(e); + + if (!response.headersSent) { + response.writeHead(500, { + "Content-Type": "application/dicom+json" + }); + return response.json(getInternalServerErrorMessage("An exception occurred")); + } + return response.end(); + } + + isCustomError(e) { + return Object.keys(ApiErrorHandlerMapping).find(key => e.name === key); + } +} + +module.exports.ApiErrorArrayHandler = ApiErrorArrayHandler; \ No newline at end of file diff --git a/error/controller.handler.js b/error/controller.handler.js deleted file mode 100644 index 364444eb..00000000 --- a/error/controller.handler.js +++ /dev/null @@ -1,24 +0,0 @@ -const { getInternalServerErrorMessage } = require("@root/utils/errorResponse/errorResponseMessage"); - -class ControllerErrorHandler { - - /** - * - * @param {Error} e - * @param {ApiLogger} apiLogger - * @param {import("express").Response} response - */ - static raiseInternalServerError(e, apiLogger, response) { - apiLogger.logger.error(e); - - if (!response.headersSent) { - response.writeHead(500, { - "Content-Type": "application/dicom+json" - }); - return response.json(getInternalServerErrorMessage("An exception occurred")); - } - return response.end(); - } -} - -module.exports.ControllerErrorHandler = ControllerErrorHandler; \ No newline at end of file