ADM-735:[backend]feat: calculate error info from three parts and not …

…throw 401,403,404 exception when generate report (#911) * ADM-735:[backend]chore: update api design * ADM-735:[backend]feat: add reportError and errorInfo class * ADM-735:[backend]feat: add reportError in response * ADM-735:[backend]feat: calculate error info from three parts and not throw 401,403,404 exception when generate report * ADM-735 update error msg * ADM-652[frontend][backend] update error message --------- Co-authored-by: Yunsong <[email protected]> Co-authored-by: guzhongren <[email protected]>
au-heartbeat · Jan 11, 2024 · b33b7ad · b33b7ad
1 parent 3d95dd9
commit b33b7ad
Show file tree

Hide file tree

Showing 13 changed files with 121 additions and 47 deletions.
diff --git a/backend/src/main/java/heartbeat/controller/report/dto/response/ErrorInfo.java b/backend/src/main/java/heartbeat/controller/report/dto/response/ErrorInfo.java
@@ -0,0 +1,18 @@
+package heartbeat.controller.report.dto.response;
+
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+@Data
+@NoArgsConstructor
+@AllArgsConstructor
+@Builder
+public class ErrorInfo {
+
+	private int status;
+
+	private String errorMessage;
+
+}
diff --git a/backend/src/main/java/heartbeat/controller/report/dto/response/ReportError.java b/backend/src/main/java/heartbeat/controller/report/dto/response/ReportError.java
@@ -0,0 +1,20 @@
+package heartbeat.controller.report.dto.response;
+
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+@Data
+@NoArgsConstructor
+@AllArgsConstructor
+@Builder
+public class ReportError {
+
+	private ErrorInfo boardError;
+
+	private ErrorInfo pipelineError;
+
+	private ErrorInfo sourceControlError;
+
+}
diff --git a/backend/src/main/java/heartbeat/controller/report/dto/response/ReportResponse.java b/backend/src/main/java/heartbeat/controller/report/dto/response/ReportResponse.java
@@ -27,6 +27,8 @@ public class ReportResponse {
 
 	private LeadTimeForChanges leadTimeForChanges;
 
+	private ReportError reportError;
+
 	private Long exportValidityTime;
 
 	private Boolean isBoardMetricsReady;

diff --git a/backend/src/main/java/heartbeat/exception/RestResponseEntityExceptionHandler.java b/backend/src/main/java/heartbeat/exception/RestResponseEntityExceptionHandler.java
@@ -26,25 +26,25 @@ protected ResponseEntity<Object> handleDecryptProcessException(DecryptDataOrPass
 	@ExceptionHandler(value = EncryptDecryptProcessException.class)
 	protected ResponseEntity<Object> handleEncryptProcessException(EncryptDecryptProcessException ex) {
 		return ResponseEntity.status(ex.getStatus())
-			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Encrypt or decrypt process failed"));
+			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Failed to encrypt or decrypt process"));
 	}
 
 	@ExceptionHandler(value = GenerateReportException.class)
 	protected ResponseEntity<Object> handleGenerateReportException(GenerateReportException ex) {
 		return ResponseEntity.status(ex.getStatus())
-			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Generate report failed"));
+			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Failed to generate report"));
 	}
 
 	@ExceptionHandler(value = NotFoundException.class)
 	protected ResponseEntity<Object> handleNotFoundException(NotFoundException ex) {
 		return ResponseEntity.status(ex.getStatus())
-			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "404 Not Found"));
+			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Not found"));
 	}
 
 	@ExceptionHandler(value = ServiceUnavailableException.class)
 	protected ResponseEntity<Object> handleTimeoutException(ServiceUnavailableException ex) {
 		return ResponseEntity.status(ex.getStatus())
-			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Service Unavailable"));
+			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Service unavailable"));
 	}
 
 	@ExceptionHandler(value = RequestFailedException.class)
@@ -106,7 +106,7 @@ public ResponseEntity<Object> handleConstraintViolation(ConstraintViolationExcep
 	@ExceptionHandler(FileIOException.class)
 	public ResponseEntity<Object> handleFileIOException(FileIOException ex) {
 		return ResponseEntity.status(ex.getStatus())
-			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "File read failed"));
+			.body(new RestApiErrorResponse(ex.getStatus(), ex.getMessage(), "Failed to read file"));
 	}
 
 	@ExceptionHandler(InternalServerErrorException.class)

diff --git a/backend/src/main/java/heartbeat/handler/AsyncExceptionHandler.java b/backend/src/main/java/heartbeat/handler/AsyncExceptionHandler.java
@@ -23,7 +23,7 @@ public void put(String reportId, BaseException e) {
 	}
 
 	public BaseException get(String reportId) {
-		return exceptionMap.remove(reportId);
+		return exceptionMap.get(reportId);
 	}
 
 	public void deleteExpireException(long currentTimeStamp) {

diff --git a/backend/src/main/java/heartbeat/service/report/GenerateReporterService.java b/backend/src/main/java/heartbeat/service/report/GenerateReporterService.java
@@ -30,18 +30,18 @@
 import heartbeat.controller.report.dto.request.JiraBoardSetting;
 import heartbeat.controller.report.dto.response.BoardCSVConfig;
 import heartbeat.controller.report.dto.response.BoardCSVConfigEnum;
+import heartbeat.controller.report.dto.response.ErrorInfo;
 import heartbeat.controller.report.dto.response.LeadTimeInfo;
 import heartbeat.controller.report.dto.response.MetricsDataReady;
 import heartbeat.controller.report.dto.response.PipelineCSVInfo;
+import heartbeat.controller.report.dto.response.ReportError;
 import heartbeat.controller.report.dto.response.ReportResponse;
 import heartbeat.exception.BadRequestException;
 import heartbeat.exception.BaseException;
 import heartbeat.exception.GenerateReportException;
 import heartbeat.exception.NotFoundException;
-import heartbeat.exception.PermissionDenyException;
 import heartbeat.exception.RequestFailedException;
 import heartbeat.exception.ServiceUnavailableException;
-import heartbeat.exception.UnauthorizedException;
 import heartbeat.handler.AsyncExceptionHandler;
 import heartbeat.handler.AsyncReportRequestHandler;
 import heartbeat.service.board.jira.JiraColumnResult;
@@ -805,24 +805,23 @@ public boolean checkGenerateReportIsDone(String reportTimeStamp) {
 		if (validateExpire(System.currentTimeMillis(), Long.parseLong(reportTimeStamp))) {
 			throw new GenerateReportException("Failed to get report due to report time expires");
 		}
-		BaseException boardException = asyncExceptionHandler.get(IdUtil.getBoardReportId(reportTimeStamp));
-		BaseException doraException = asyncExceptionHandler.get(IdUtil.getPipelineReportId(reportTimeStamp));
-		handleAsyncException(boardException);
-		handleAsyncException(doraException);
 		return asyncReportRequestHandler.isReportReady(reportTimeStamp);
 	}
 
-	private static void handleAsyncException(BaseException exception) {
+	private ErrorInfo handleAsyncExceptionAndGetErrorInfo(BaseException exception) {
 		if (Objects.nonNull(exception)) {
-			switch (exception.getStatus()) {
-				case 401 -> throw new UnauthorizedException(exception.getMessage());
-				case 403 -> throw new PermissionDenyException(exception.getMessage());
-				case 404 -> throw new NotFoundException(exception.getMessage());
-				case 500 -> throw new GenerateReportException(exception.getMessage());
-				case 503 -> throw new ServiceUnavailableException(exception.getMessage());
-				default -> throw new RequestFailedException(exception.getStatus(), exception.getMessage());
+			int status = exception.getStatus();
+			final String errorMessage = exception.getMessage();
+			switch (status) {
+				case 401, 403, 404 -> {
+					return ErrorInfo.builder().status(status).errorMessage(errorMessage).build();
+				}
+				case 500 -> throw new GenerateReportException(errorMessage);
+				case 503 -> throw new ServiceUnavailableException(errorMessage);
+				default -> throw new RequestFailedException(status, errorMessage);
 			}
 		}
+		return null;
 	}
 
 	private void validateExpire(long csvTimeStamp) {
@@ -873,6 +872,7 @@ public ReportResponse getComposedReportResponse(String reportId, boolean isRepor
 		ReportResponse codebaseReportResponse = getReportFromHandler(IdUtil.getSourceControlReportId(reportId));
 		MetricsDataReady metricsDataReady = asyncReportRequestHandler.getMetricsDataReady(reportId);
 		ReportResponse response = Optional.ofNullable(boardReportResponse).orElse(doraReportResponse);
+		ReportError reportError = getReportErrorAndHandleAsyncException(reportId);
 
 		return ReportResponse.builder()
 			.velocity(getValueOrNull(boardReportResponse, ReportResponse::getVelocity))
@@ -888,6 +888,21 @@ public ReportResponse getComposedReportResponse(String reportId, boolean isRepor
 			.isSourceControlMetricsReady(
 					getValueOrNull(metricsDataReady, MetricsDataReady::isSourceControlMetricsReady))
 			.isAllMetricsReady(isReportReady)
+			.reportError(reportError)
+			.build();
+	}
+
+	public ReportError getReportErrorAndHandleAsyncException(String reportId) {
+		BaseException boardException = asyncExceptionHandler.get(IdUtil.getBoardReportId(reportId));
+		BaseException pipelineException = asyncExceptionHandler.get(IdUtil.getPipelineReportId(reportId));
+		BaseException sourceControlException = asyncExceptionHandler.get(IdUtil.getSourceControlReportId(reportId));
+		ErrorInfo boardError = handleAsyncExceptionAndGetErrorInfo(boardException);
+		ErrorInfo pipelineError = handleAsyncExceptionAndGetErrorInfo(pipelineException);
+		ErrorInfo sourceControlError = handleAsyncExceptionAndGetErrorInfo(sourceControlException);
+		return ReportError.builder()
+			.boardError(boardError)
+			.pipelineError(pipelineError)
+			.sourceControlError(sourceControlError)
 			.build();
 	}
 

diff --git a/backend/src/test/java/heartbeat/controller/crypto/CryptoControllerTest.java b/backend/src/test/java/heartbeat/controller/crypto/CryptoControllerTest.java
@@ -137,7 +137,7 @@ void shouldReturn500StatusWhenServiceThrowException() throws Exception {
 		final var message = JsonPath.parse(content).read("$.message").toString();
 		final var hintInfo = JsonPath.parse(content).read("$.hintInfo").toString();
 		assertThat(message).isEqualTo(FAKE_EXCEPTION_MESSAGE);
-		assertThat(hintInfo).isEqualTo("Encrypt or decrypt process failed");
+		assertThat(hintInfo).isEqualTo("Failed to encrypt or decrypt process");
 	}
 
 	@Test
@@ -224,7 +224,7 @@ void shouldReturn5xxOr4xxWhenDecryptServiceThrowException() throws Exception {
 		final var internalServerMessage = JsonPath.parse(internalServerContent).read("$.message").toString();
 		final var internalServerHintInfo = JsonPath.parse(internalServerContent).read("$.hintInfo").toString();
 		assertThat(internalServerMessage).isEqualTo(FAKE_EXCEPTION_MESSAGE);
-		assertThat(internalServerHintInfo).isEqualTo("Encrypt or decrypt process failed");
+		assertThat(internalServerHintInfo).isEqualTo("Failed to encrypt or decrypt process");
 
 		var badRequestResponse = mockMvc
 			.perform(post("/decrypt").content(new ObjectMapper().writeValueAsString(request))

diff --git a/backend/src/test/java/heartbeat/controller/report/GenerateReporterControllerTest.java b/backend/src/test/java/heartbeat/controller/report/GenerateReporterControllerTest.java
@@ -109,7 +109,7 @@ void shouldReturnInternalServerErrorStatusWhenCheckGenerateReportThrowException(
 		final var hintInfo = JsonPath.parse(content).read("$.hintInfo").toString();
 
 		assertEquals("Report time expires", errorMessage);
-		assertEquals("Generate report failed", hintInfo);
+		assertEquals("Failed to generate report", hintInfo);
 	}
 
 	@Test

diff --git a/backend/src/test/java/heartbeat/service/report/GenerateReporterServiceTest.java b/backend/src/test/java/heartbeat/service/report/GenerateReporterServiceTest.java
@@ -27,6 +27,7 @@
 import heartbeat.controller.report.dto.response.ClassificationNameValuePair;
 import heartbeat.controller.report.dto.response.CycleTime;
 import heartbeat.controller.report.dto.response.DeploymentFrequency;
+import heartbeat.controller.report.dto.response.ErrorInfo;
 import heartbeat.controller.report.dto.response.LeadTimeForChanges;
 import heartbeat.controller.report.dto.response.LeadTimeForChangesOfPipelines;
 import heartbeat.controller.report.dto.response.MeanTimeToRecovery;
@@ -973,11 +974,11 @@ void shouldThrowUnauthorizedExceptionWhenCheckGenerateReportIsDone() {
 		when(asyncExceptionHandler.get(reportId))
 			.thenReturn(new UnauthorizedException("Failed to get GitHub info_status: 401, reason: PermissionDeny"));
 
-		BaseException exception = assertThrows(UnauthorizedException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+		ErrorInfo pipelineError = generateReporterService.getReportErrorAndHandleAsyncException(timeStamp)
+			.getPipelineError();
 
-		assertEquals(401, exception.getStatus());
-		assertEquals("Failed to get GitHub info_status: 401, reason: PermissionDeny", exception.getMessage());
+		assertEquals(401, pipelineError.getStatus());
+		assertEquals("Failed to get GitHub info_status: 401, reason: PermissionDeny", pipelineError.getErrorMessage());
 	}
 
 	@Test
@@ -989,11 +990,11 @@ void shouldThrowPermissionDenyExceptionWhenCheckGenerateReportIsDone() {
 
 		when(asyncExceptionHandler.get(reportId))
 			.thenReturn(new PermissionDenyException("Failed to get GitHub info_status: 403, reason: PermissionDeny"));
-		BaseException exception = assertThrows(PermissionDenyException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+		ErrorInfo pipelineError = generateReporterService.getReportErrorAndHandleAsyncException(timeStamp)
+			.getPipelineError();
 
-		assertEquals(403, exception.getStatus());
-		assertEquals("Failed to get GitHub info_status: 403, reason: PermissionDeny", exception.getMessage());
+		assertEquals(403, pipelineError.getStatus());
+		assertEquals("Failed to get GitHub info_status: 403, reason: PermissionDeny", pipelineError.getErrorMessage());
 	}
 
 	@Test
@@ -1003,11 +1004,11 @@ void shouldThrowNotFoundExceptionWhenCheckGenerateReportIsDone() {
 
 		when(asyncExceptionHandler.get(reportId))
 			.thenReturn(new NotFoundException("Failed to get GitHub info_status: 404, reason: NotFound"));
-		BaseException exception = assertThrows(NotFoundException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+		ErrorInfo pipelineError = generateReporterService.getReportErrorAndHandleAsyncException(timeStamp)
+			.getPipelineError();
 
-		assertEquals(404, exception.getStatus());
-		assertEquals("Failed to get GitHub info_status: 404, reason: NotFound", exception.getMessage());
+		assertEquals(404, pipelineError.getStatus());
+		assertEquals("Failed to get GitHub info_status: 404, reason: NotFound", pipelineError.getErrorMessage());
 	}
 
 	@Test
@@ -1018,7 +1019,7 @@ void shouldThrowGenerateReportExceptionWhenCheckGenerateReportIsDone() {
 		when(asyncExceptionHandler.get(reportId))
 			.thenReturn(new GenerateReportException("Failed to get GitHub info_status: 500, reason: GenerateReport"));
 		BaseException exception = assertThrows(GenerateReportException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+				() -> generateReporterService.getReportErrorAndHandleAsyncException(timeStamp));
 
 		assertEquals(500, exception.getStatus());
 		assertEquals("Failed to get GitHub info_status: 500, reason: GenerateReport", exception.getMessage());
@@ -1032,7 +1033,7 @@ void shouldThrowServiceUnavailableExceptionWhenCheckGenerateReportIsDone() {
 		when(asyncExceptionHandler.get(reportId)).thenReturn(
 				new ServiceUnavailableException("Failed to get GitHub info_status: 503, reason: ServiceUnavailable"));
 		BaseException exception = assertThrows(ServiceUnavailableException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+				() -> generateReporterService.getReportErrorAndHandleAsyncException(timeStamp));
 
 		assertEquals(503, exception.getStatus());
 		assertEquals("Failed to get GitHub info_status: 503, reason: ServiceUnavailable", exception.getMessage());
@@ -1045,7 +1046,7 @@ void shouldThrowRequestFailedExceptionWhenCheckGenerateReportIsDone() {
 
 		when(asyncExceptionHandler.get(reportId)).thenReturn(new RequestFailedException(405, "RequestFailedException"));
 		BaseException exception = assertThrows(RequestFailedException.class,
-				() -> generateReporterService.checkGenerateReportIsDone(timeStamp));
+				() -> generateReporterService.getReportErrorAndHandleAsyncException(timeStamp));
 
 		assertEquals(405, exception.getStatus());
 		assertEquals(

diff --git a/docs/src/content/docs/en/designs/refinement-on-generate-report.mdx b/docs/src/content/docs/en/designs/refinement-on-generate-report.mdx
@@ -153,7 +153,7 @@ Exception Table
   <tr>
     <td rowspan="3">404</td>
     <td>Failed to get BuildKite info_status: 404...</td>
-    <td rowspan="3">404 Not Found</td>
+    <td rowspan="3">Not found</td>
   </tr>
   <tr>
     <td>Failed to get GitHub info_status: 404...</td>
@@ -164,7 +164,7 @@ Exception Table
   <tr>
     <td rowspan="3">500</td>
     <td>Report time expires</td>
-    <td rowspan="3">Generate report failed</td>
+    <td rowspan="3">Failed to generate report</td>
   </tr>
   <tr>
     <td>Failed to write report file</td>
@@ -175,7 +175,7 @@ Exception Table
   <tr>
     <td rowspan="3">503</td>
     <td>Failed to get BuildKite info_status: 503...</td>
-    <td rowspan="3">Service Unavailable</td>
+    <td rowspan="3">Service unavailable</td>
   </tr>
   <tr>
     <td>Failed to get GitHub info_status: 503...</td>
@@ -208,7 +208,7 @@ group async process board related metrics
     activate Jira
     Jira --> Backend: Jira raw 
     deactivate Jira
-    Backend -> Backend: calculate metrics for velocity, cicle time and classification
+    Backend -> Backend: calculate metrics for velocity, cycle time and classification
     Backend -> Backend: generate board report csv
     deactivate Backend
     note left
@@ -479,6 +479,20 @@ Response:
         "totalDelayTime": 0
       }
     } <could be null if not required>,
+    "reportError": {
+      "boardError": {
+        "status": 400,
+        "errorMessage": "string"
+      },
+      "pipelineError": {
+        "status": 400,
+        "errorMessage": "string"
+      },
+      "sourceControlError": {
+        "status": 400,
+        "errorMessage": "string"
+      },
+    } <could be null if not required>,
     "isBoardMetricsReady": Boolean <default is null>, 
     "isPipelineMetricsReady": Boolean <default is null>, 
     "isSourceControlMetricsReady": Boolean <default is null>,

diff --git a/frontend/__tests__/src/client/CSVClient.test.ts b/frontend/__tests__/src/client/CSVClient.test.ts
@@ -30,7 +30,11 @@ describe('verify export csv', () => {
   });
 
   it('should throw error when export csv request status 500', async () => {
-    server.use(rest.get(MOCK_EXPORT_CSV_URL, (req, res, ctx) => res(ctx.status(HttpStatusCode.InternalServerError))));
+    server.use(
+      rest.get(MOCK_EXPORT_CSV_URL, (req, res, ctx) =>
+        res(ctx.status(HttpStatusCode.InternalServerError, VERIFY_ERROR_MESSAGE.INTERNAL_SERVER_ERROR))
+      )
+    );
 
     await expect(async () => {
       await csvClient.exportCSVData(MOCK_EXPORT_CSV_REQUEST_PARAMS);

diff --git a/frontend/__tests__/src/fixtures.ts b/frontend/__tests__/src/fixtures.ts
@@ -105,8 +105,8 @@ export const VERSION_RESPONSE = {
 export enum VERIFY_ERROR_MESSAGE {
   BAD_REQUEST = 'Please reconfirm the input',
   UNAUTHORIZED = 'Token is incorrect',
-  INTERNAL_SERVER_ERROR = 'Internal Server Error',
-  NOT_FOUND = '404 Not Found',
+  INTERNAL_SERVER_ERROR = 'Internal server error',
+  NOT_FOUND = 'Not found',
   PERMISSION_DENIED = 'Permission denied',
   REQUEST_TIMEOUT = 'Request Timeout',
   UNKNOWN = 'Unknown',
@@ -692,7 +692,7 @@ export const CLASSIFICATION_WARNING_MESSAGE = `Some classifications in import da
 export const HOME_VERIFY_IMPORT_WARNING_MESSAGE =
   'The content of the imported JSON file is empty. Please confirm carefully';
 
-export const INTERNAL_SERVER_ERROR_MESSAGE = 'Internal Server Error';
+export const INTERNAL_SERVER_ERROR_MESSAGE = 'Internal server error';
 
 export const BASE_PAGE_ROUTE = '/';
 

diff --git a/frontend/cypress/pages/metrics/metrics.ts b/frontend/cypress/pages/metrics/metrics.ts
@@ -76,7 +76,7 @@ export class Metrics {
   }
 
   get buildKiteStepNotFoundTips() {
-    return cy.contains('BuildKite get steps failed: 404 Not Found');
+    return cy.contains('BuildKite get steps failed: Not found');
   }
 
   get pipelineRemoveButton() {