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

ADM-735:[backend]feat: calculate error info from three parts and not throw 401,403,404 exception when generate report #911

Merged
merged 8 commits into from
Jan 11, 2024
Original file line number Diff line number Diff line change
@@ -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;

}
Original file line number Diff line number Diff line change
@@ -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;

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public class ReportResponse {

private LeadTimeForChanges leadTimeForChanges;

private ReportError reportError;

private Long exportValidityTime;

private Boolean isBoardMetricsReady;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
Expand All @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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(
Expand Down
22 changes: 18 additions & 4 deletions docs/src/content/docs/en/designs/refinement-on-generate-report.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Expand Down
6 changes: 5 additions & 1 deletion frontend/__tests__/src/client/CSVClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions frontend/__tests__/src/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 = '/';

Expand Down
2 changes: 1 addition & 1 deletion frontend/cypress/pages/metrics/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading