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.

Sign up for GitHub

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-999 [frontend][backend]: fix the error path #1598

Merged
merged 2 commits into from
Sep 6, 2024
Merged

ADM-999 [frontend][backend]: fix the error path #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
398e550
ADM-999 [frontend][backend]: fix the error path
zhou-yinyuan Sep 5, 2024
b32834d
ADM-999 [frontend][backend]: fix the test
zhou-yinyuan Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 26 additions & 12 deletions backend/src/main/java/heartbeat/service/pipeline/buildkite/CachePageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
import heartbeat.client.dto.pipeline.buildkite.BuildKiteBuildInfo;
import heartbeat.client.dto.pipeline.buildkite.PageBuildKitePipelineInfoDTO;
import heartbeat.client.dto.pipeline.buildkite.PageStepsInfoDto;
import heartbeat.exception.BaseException;
import heartbeat.exception.InternalServerErrorException;
import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -81,10 +83,13 @@ public PageOrganizationsInfoDTO getGitHubOrganizations(String token, int page, i
int totalPage = parseTotalPage(allOrganizations.getHeaders().get(BUILD_KITE_LINK_HEADER));
return PageOrganizationsInfoDTO.builder().pageInfo(allOrganizations.getBody()).totalPage(totalPage).build();
}
catch (Exception e) {
catch (BaseException e) {
log.info("Error to get paginated github organization info, page: {}, exception: {}", page, e);
throw new InternalServerErrorException(
String.format("Error to get paginated github organization info, page: %s, exception: %s", page, e));
if (e.getStatus() == HttpStatus.INTERNAL_SERVER_ERROR.value()) {
throw new InternalServerErrorException(String
.format("Error to get paginated github organization info, page: %s, exception: %s", page, e));
}
throw e;
}
}

Expand All @@ -98,10 +103,13 @@ public PageReposInfoDTO getGitHubRepos(String token, String organization, int pa
int totalPage = parseTotalPage(allRepos.getHeaders().get(BUILD_KITE_LINK_HEADER));
return PageReposInfoDTO.builder().pageInfo(allRepos.getBody()).totalPage(totalPage).build();
}
catch (Exception e) {
catch (BaseException e) {
log.info("Error to get paginated github repo info, page: {}, exception: {}", page, e);
throw new InternalServerErrorException(
String.format("Error to get paginated github repo info, page: %s, exception: %s", page, e));
if (e.getStatus() == HttpStatus.INTERNAL_SERVER_ERROR.value()) {
throw new InternalServerErrorException(
String.format("Error to get paginated github repo info, page: %s, exception: %s", page, e));
}
throw e;
}
}

Expand All @@ -116,10 +124,13 @@ public PageBranchesInfoDTO getGitHubBranches(String token, String organization,
int totalPage = parseTotalPage(allRepos.getHeaders().get(BUILD_KITE_LINK_HEADER));
return PageBranchesInfoDTO.builder().pageInfo(allRepos.getBody()).totalPage(totalPage).build();
}
catch (Exception e) {
catch (BaseException e) {
log.info("Error to get paginated github branch info, page: {}, exception: {}", page, e);
throw new InternalServerErrorException(
String.format("Error to get paginated github branch info, page: %s, exception: %s", page, e));
if (e.getStatus() == HttpStatus.INTERNAL_SERVER_ERROR.value()) {
throw new InternalServerErrorException(
String.format("Error to get paginated github branch info, page: %s, exception: %s", page, e));
}
throw e;
}
}

Expand All @@ -135,10 +146,13 @@ public PagePullRequestInfoDTO getGitHubPullRequest(String token, String organiza
int totalPage = parseTotalPage(allPullRequests.getHeaders().get(BUILD_KITE_LINK_HEADER));
return PagePullRequestInfoDTO.builder().pageInfo(allPullRequests.getBody()).totalPage(totalPage).build();
}
catch (Exception e) {
catch (BaseException e) {
log.info("Error to get paginated github pull request info, page: {}, exception: {}", page, e);
throw new InternalServerErrorException(
String.format("Error to get paginated github pull request info, page: %s, exception: %s", page, e));
if (e.getStatus() == HttpStatus.INTERNAL_SERVER_ERROR.value()) {
throw new InternalServerErrorException(String
.format("Error to get paginated github pull request info, page: %s, exception: %s", page, e));
}
throw e;
}
}

Expand Down
96 changes: 88 additions & 8 deletions backend/src/test/java/heartbeat/service/pipeline/buildkite/CachePageServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@
import heartbeat.client.dto.pipeline.buildkite.BuildKiteJob;
import heartbeat.client.dto.pipeline.buildkite.BuildKitePipelineDTO;
import heartbeat.client.dto.pipeline.buildkite.PageStepsInfoDto;
import heartbeat.exception.BaseException;
import heartbeat.exception.InternalServerErrorException;
import heartbeat.exception.NotFoundException;
import heartbeat.exception.PermissionDenyException;
import heartbeat.exception.RequestFailedException;
import heartbeat.exception.UnauthorizedException;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
Expand All @@ -32,6 +40,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hibernate.validator.internal.util.Contracts.assertNotNull;
Expand Down Expand Up @@ -218,11 +227,27 @@ void shouldReturnPageOrganizationsInfoDtoWhenFetchPageOrganizationsInfoSuccessGi

@Test
void shouldThrowExceptionWhenFetchPageOrganizationsInfoThrow500() {
when(gitHubFeignClient.getAllOrganizations(MOCK_TOKEN, 100, 1)).thenThrow(RuntimeException.class);
when(gitHubFeignClient.getAllOrganizations(MOCK_TOKEN, 100, 1)).thenThrow(new RequestFailedException(500, "error"));

InternalServerErrorException internalServerErrorException = assertThrows(InternalServerErrorException.class, () -> cachePageService.getGitHubOrganizations(MOCK_TOKEN, 1, 100));

assertEquals(500, internalServerErrorException.getStatus());
assertEquals("Error to get paginated github organization info, page: 1, exception: java.lang.RuntimeException", internalServerErrorException.getMessage());
assertEquals("Error to get paginated github organization info, page: 1, exception: heartbeat.exception.RequestFailedException: Request failed with status statusCode 500, error: error",
internalServerErrorException.getMessage());
}

@ParameterizedTest
@MethodSource("baseExceptionProvider")
void shouldThrowExceptionWhenFetchPageOrganizationInfoThrow4xx(BaseException e, int errorCode) {
when(gitHubFeignClient.getAllOrganizations(MOCK_TOKEN, 100, 1))
.thenThrow(e);

BaseException baseException = assertThrows(BaseException.class,
() -> cachePageService.getGitHubOrganizations(MOCK_TOKEN, 1, 100));

assertEquals(errorCode, baseException.getStatus());
assertEquals("error",
baseException.getMessage());
}

@Test
Expand All @@ -243,15 +268,31 @@ void shouldReturnPageReposInfoDtoWhenFetchPageReposInfoSuccessGivenExist() throw
@Test
void shouldThrowExceptionWhenFetchPageRepoInfoThrow500() {
String organization = "test-org";
when(gitHubFeignClient.getAllRepos(MOCK_TOKEN, organization, 100, 1)).thenThrow(RuntimeException.class);
when(gitHubFeignClient.getAllRepos(MOCK_TOKEN, organization, 100, 1))
.thenThrow(new RequestFailedException(500, "error"));

InternalServerErrorException internalServerErrorException = assertThrows(InternalServerErrorException.class,
() -> cachePageService.getGitHubRepos(MOCK_TOKEN, organization, 1, 100));

assertEquals(500, internalServerErrorException.getStatus());
assertEquals("Error to get paginated github repo info, page: 1, exception: java.lang.RuntimeException",
assertEquals(
"Error to get paginated github repo info, page: 1, exception: heartbeat.exception.RequestFailedException: Request failed with status statusCode 500, error: error",
internalServerErrorException.getMessage());
}

@ParameterizedTest
@MethodSource("baseExceptionProvider")
void shouldThrowExceptionWhenFetchPageRepoInfoThrow4xx(BaseException e, int errorCode) {
String organization = "test-org";
when(gitHubFeignClient.getAllRepos(MOCK_TOKEN, organization, 100, 1)).thenThrow(e);

BaseException baseException = assertThrows(BaseException.class,
() -> cachePageService.getGitHubRepos(MOCK_TOKEN, organization, 1, 100));

assertEquals(errorCode, baseException.getStatus());
assertEquals("error", baseException.getMessage());
}

@Test
void shouldReturnPageBranchesInfoDtoWhenFetchPageBranchesInfoSuccessGivenExist() throws IOException {
String organization = "test-org";
Expand All @@ -274,15 +315,31 @@ void shouldThrowExceptionWhenFetchPageBranchInfoThrow500() {
String organization = "test-org";
String repo = "test-repo";
when(gitHubFeignClient.getAllBranches(MOCK_TOKEN, organization, repo, 100, 1))
.thenThrow(RuntimeException.class);
.thenThrow(new RequestFailedException(500, "error"));

InternalServerErrorException internalServerErrorException = assertThrows(InternalServerErrorException.class,
() -> cachePageService.getGitHubBranches(MOCK_TOKEN, organization, repo, 1, 100));

assertEquals(500, internalServerErrorException.getStatus());
assertEquals("Error to get paginated github branch info, page: 1, exception: java.lang.RuntimeException",
assertEquals(
"Error to get paginated github branch info, page: 1, exception: heartbeat.exception.RequestFailedException: Request failed with status statusCode 500, error: error",
internalServerErrorException.getMessage());
}

@ParameterizedTest
@MethodSource("baseExceptionProvider")
void shouldThrowExceptionWhenFetchPageBranchInfoThrow4xx(BaseException e, int errorCode) {
String organization = "test-org";
String repo = "test-repo";
when(gitHubFeignClient.getAllBranches(MOCK_TOKEN, organization, repo, 100, 1)).thenThrow(e);

BaseException baseException = assertThrows(BaseException.class,
() -> cachePageService.getGitHubBranches(MOCK_TOKEN, organization, repo, 1, 100));

assertEquals(errorCode, baseException.getStatus());
assertEquals("error", baseException.getMessage());
}

@Test
void shouldReturnPagePullRequestInfoDtoWhenFetchPullRequestInfoSuccessGivenExist() throws IOException {
String organization = "test-org";
Expand All @@ -308,15 +365,31 @@ void shouldThrowExceptionWhenFetchPagePullRequestInfoThrow500() {
String repo = "test-repo";
String branch = "test-branch";
when(gitHubFeignClient.getAllPullRequests(MOCK_TOKEN, organization, repo, 100, 1, branch, "all"))
.thenThrow(RuntimeException.class);
.thenThrow(new RequestFailedException(500, "error"));

InternalServerErrorException internalServerErrorException = assertThrows(InternalServerErrorException.class,
() -> cachePageService.getGitHubPullRequest(MOCK_TOKEN, organization, repo, branch, 1, 100));
assertEquals(500, internalServerErrorException.getStatus());
assertEquals("Error to get paginated github pull request info, page: 1, exception: java.lang.RuntimeException",
assertEquals(
"Error to get paginated github pull request info, page: 1, exception: heartbeat.exception.RequestFailedException: Request failed with status statusCode 500, error: error",
internalServerErrorException.getMessage());
}

@ParameterizedTest
@MethodSource("baseExceptionProvider")
void shouldThrowExceptionWhenFetchPagePullRequestInfoThrow4xx(BaseException e, int errorCode) {
String organization = "test-org";
String repo = "test-repo";
String branch = "test-branch";
when(gitHubFeignClient.getAllPullRequests(MOCK_TOKEN, organization, repo, 100, 1, branch, "all")).thenThrow(e);

BaseException baseException = assertThrows(BaseException.class,
() -> cachePageService.getGitHubPullRequest(MOCK_TOKEN, organization, repo, branch, 1, 100));

assertEquals(errorCode, baseException.getStatus());
assertEquals("error", baseException.getMessage());
}

private static <T> ResponseEntity<List<T>> getResponseEntity(HttpHeaders httpHeaders, String pathname)
throws IOException {
ObjectMapper mapper = new ObjectMapper();
Expand All @@ -337,4 +410,11 @@ private HttpHeaders buildGitHubHttpHeaders() {
return buildHttpHeaders(GITHUB_TOTAL_PAGE_HEADER);
}

static Stream<Arguments> baseExceptionProvider() {
return Stream.of(Arguments.of(new PermissionDenyException("error"), 403),
Arguments.of(new UnauthorizedException("error"), 401), Arguments.of(new NotFoundException("error"), 404)

);
}

}
36 changes: 36 additions & 0 deletions ...s__/containers/MetricsStep/SourceControlConfiguration/SourceControlConfiguration.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { IUseGetSourceControlConfigurationCrewInterface } from '@src/hooks/useGe
import { SourceControlConfiguration } from '@src/containers/MetricsStep/SouceControlConfiguration';
import { act, render, screen, waitFor, within } from '@testing-library/react';
import { LIST_OPEN, LOADING, REMOVE_BUTTON } from '@test/fixtures';
import { MetricsDataFailStatus } from '@src/constants/commons';
import { setupStore } from '@test/utils/setupStoreUtil';
import userEvent from '@testing-library/user-event';
import { Provider } from 'react-redux';
Expand All @@ -35,16 +36,37 @@ const mockInitRepoEffectResponse = {
isGetRepo: true,
isLoading: false,
getSourceControlRepoInfo: jest.fn(),
info: {
code: 200,
data: undefined,
errorTitle: '',
errorMessage: '',
},
stepFailedStatus: MetricsDataFailStatus.NotFailed,
};
const mockInitBranchEffectResponse = {
isLoading: false,
getSourceControlBranchInfo: jest.fn(),
isGetBranch: true,
info: {
code: 200,
data: undefined,
errorTitle: '',
errorMessage: '',
},
stepFailedStatus: MetricsDataFailStatus.NotFailed,
};
const mockInitCrewEffectResponse = {
isLoading: false,
getSourceControlCrewInfo: jest.fn(),
isGetAllCrews: true,
info: {
code: 200,
data: undefined,
errorTitle: '',
errorMessage: '',
},
stepFailedStatus: MetricsDataFailStatus.NotFailed,
};

let mockSourceControlSettings = mockInitSourceControlSettings;
Expand Down Expand Up @@ -221,4 +243,18 @@ describe('SourceControlConfiguration', () => {

expect(screen.getByText('Crew setting (optional)')).toBeInTheDocument();
});

it('should set error info when any request return error', () => {
mockOrganizationEffectResponse = {
...mockOrganizationEffectResponse,
info: {
code: 404,
errorTitle: 'error title',
errorMessage: 'error message',
},
};
setup();

expect(screen.getByLabelText('Error UI for pipeline settings')).toBeInTheDocument();
});
});
Loading
Loading