Skip to content

Commit 622c7bf

Browse files
aindriu-aivenaindriu-aiven
authored
Issue better error responses to UI (#1133)
* Do a look up for the superadmin or admin to run the queries for migration to 2.0.0 Klaw Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Initial commit for better error response handling Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Return the correct error messages Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to responses Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to responses Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to responses Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to reduce duplication Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Additional exception handling to ensure that errors do not escape Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to address PRs Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to address PRs Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Update to address PRs Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Special character introduced in error messages had to be removed. Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * Special character introduced in error messages had to be removed. Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> * run spotless Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io> --------- Signed-off-by: Aindriu Lavelle <aindriu.lavelle@aiven.io>
1 parent 4a653d0 commit 622c7bf

16 files changed

Lines changed: 415 additions & 21 deletions

File tree

‎cluster-api/src/main/java/io/aiven/klaw/clusterapi/controller/KafkaConnectController.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public ResponseEntity<ApiResponse> postConnector(
6363
return new ResponseEntity<>(result, HttpStatus.OK);
6464
} catch (Exception e) {
6565
return new ResponseEntity<>(
66-
ApiResponse.builder().success(false).message("Unable to register connector").build(),
66+
ApiResponse.builder().success(false).message(e.getMessage()).build(),
6767
HttpStatus.INTERNAL_SERVER_ERROR);
6868
}
6969
}

‎cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/ClusterApiErrorMessages.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package io.aiven.klaw.clusterapi.models.error;
2+
3+
public class ClusterApiErrorMessages {
4+
5+
public static final String CLUSTER_API_ERR_1 = "Unable to create Connector on Cluster.";
6+
7+
public static final String CLUSTER_API_ERR_2 = "Unable to update Connector on Cluster";
8+
9+
public static final String CLUSTER_API_ERR_3 = "Unable To Delete Connector on Cluster.";
10+
}

‎cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/RestErrorResponse.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.aiven.klaw.clusterapi.models.error;
2+
3+
import com.fasterxml.jackson.annotation.JsonAlias;
4+
import lombok.Data;
5+
6+
@Data
7+
public class RestErrorResponse {
8+
9+
private String message;
10+
11+
@JsonAlias("error_code")
12+
private int errorCode;
13+
}

‎cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/KafkaConnectService.java‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package io.aiven.klaw.clusterapi.services;
22

3+
import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_1;
4+
import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_2;
5+
import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_3;
6+
37
import io.aiven.klaw.clusterapi.models.ApiResponse;
48
import io.aiven.klaw.clusterapi.models.ClusterConnectorRequest;
59
import io.aiven.klaw.clusterapi.models.enums.ApiResultStatus;
610
import io.aiven.klaw.clusterapi.models.enums.ClusterStatus;
711
import io.aiven.klaw.clusterapi.models.enums.KafkaClustersType;
812
import io.aiven.klaw.clusterapi.models.enums.KafkaSupportedProtocol;
13+
import io.aiven.klaw.clusterapi.models.error.RestErrorResponse;
914
import io.aiven.klaw.clusterapi.utils.ClusterApiUtils;
1015
import java.util.Collections;
1116
import java.util.HashMap;
@@ -19,6 +24,9 @@
1924
import org.springframework.http.HttpMethod;
2025
import org.springframework.http.ResponseEntity;
2126
import org.springframework.stereotype.Service;
27+
import org.springframework.web.client.HttpClientErrorException;
28+
import org.springframework.web.client.HttpServerErrorException;
29+
import org.springframework.web.client.HttpStatusCodeException;
2230
import org.springframework.web.client.RestClientException;
2331
import org.springframework.web.client.RestTemplate;
2432

@@ -58,9 +66,12 @@ public ApiResponse deleteConnector(ClusterConnectorRequest clusterConnectorReque
5866
HttpMethod.DELETE,
5967
request,
6068
new ParameterizedTypeReference<>() {});
61-
} catch (RestClientException e) {
69+
} catch (HttpServerErrorException | HttpClientErrorException e) {
6270
log.error("Error in deleting connector ", e);
63-
return ApiResponse.builder().success(false).message(e.getMessage()).build();
71+
return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_3);
72+
} catch (RestClientException ex) {
73+
log.error("Error in deleting connector ", ex);
74+
return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_3).build();
6475
}
6576
return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build();
6677
}
@@ -84,13 +95,30 @@ public ApiResponse updateConnector(ClusterConnectorRequest clusterConnectorReque
8495

8596
try {
8697
reqDetails.getRight().put(reqDetails.getLeft(), request, String.class);
87-
} catch (RestClientException e) {
98+
} catch (HttpServerErrorException | HttpClientErrorException e) {
8899
log.error("Error in updating connector ", e);
89-
return ApiResponse.builder().success(false).message(e.getMessage()).build();
100+
return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_2);
101+
} catch (Exception ex) {
102+
return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_2).build();
90103
}
91104
return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build();
92105
}
93106

107+
private static ApiResponse buildErrorResponseFromRestException(
108+
HttpStatusCodeException e, String defaultErrorMsg) {
109+
RestErrorResponse errorResponse = null;
110+
try {
111+
errorResponse = e.getResponseBodyAs(RestErrorResponse.class);
112+
} catch (Exception ex) {
113+
log.error("Error caught trying to process the error response. ", ex);
114+
}
115+
if (errorResponse != null) {
116+
return ApiResponse.builder().success(false).message(errorResponse.getMessage()).build();
117+
} else {
118+
return ApiResponse.builder().success(false).message(defaultErrorMsg).build();
119+
}
120+
}
121+
94122
public ApiResponse postNewConnector(ClusterConnectorRequest clusterConnectorRequest)
95123
throws Exception {
96124
log.info("Into postNewConnector clusterConnectorRequest {} ", clusterConnectorRequest);
@@ -110,9 +138,11 @@ public ApiResponse postNewConnector(ClusterConnectorRequest clusterConnectorRequ
110138
try {
111139
responseNew =
112140
reqDetails.getRight().postForEntity(reqDetails.getLeft(), request, String.class);
113-
} catch (RestClientException e) {
114-
log.error("Error in registering new connector ", e);
115-
throw new Exception(e.toString());
141+
} catch (HttpServerErrorException | HttpClientErrorException e) {
142+
143+
return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_1);
144+
} catch (Exception ex) {
145+
return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_1).build();
116146
}
117147
if (responseNew.getStatusCodeValue() == 201) {
118148
return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build();

‎cluster-api/src/test/java/io/aiven/klaw/clusterapi/services/KafkaConnectServiceTest.java‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@
55
import static org.mockito.ArgumentMatchers.eq;
66
import static org.mockito.Mockito.when;
77
import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
8+
import static org.springframework.test.web.client.response.MockRestResponseCreators.withBadRequest;
9+
import static org.springframework.test.web.client.response.MockRestResponseCreators.withRawStatus;
810
import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
911

1012
import com.fasterxml.jackson.core.JsonProcessingException;
1113
import com.fasterxml.jackson.databind.ObjectMapper;
14+
import io.aiven.klaw.clusterapi.models.ApiResponse;
15+
import io.aiven.klaw.clusterapi.models.ClusterConnectorRequest;
16+
import io.aiven.klaw.clusterapi.models.enums.ApiResultStatus;
17+
import io.aiven.klaw.clusterapi.models.enums.KafkaClustersType;
1218
import io.aiven.klaw.clusterapi.models.enums.KafkaSupportedProtocol;
19+
import io.aiven.klaw.clusterapi.models.error.RestErrorResponse;
1320
import io.aiven.klaw.clusterapi.utils.ClusterApiUtils;
1421
import java.util.Collections;
1522
import org.apache.commons.lang3.tuple.Pair;
@@ -19,13 +26,16 @@
1926
import org.springframework.beans.factory.annotation.Autowired;
2027
import org.springframework.boot.test.autoconfigure.web.client.RestClientTest;
2128
import org.springframework.boot.test.mock.mockito.MockBean;
29+
import org.springframework.http.HttpHeaders;
2230
import org.springframework.http.MediaType;
2331
import org.springframework.test.web.client.MockRestServiceServer;
2432
import org.springframework.web.client.RestTemplate;
2533

2634
@RestClientTest(KafkaConnectService.class)
2735
class KafkaConnectServiceTest {
2836

37+
public static final String THIS_IS_A_MISCONFIGURED_CONNECTOR =
38+
"This is a misconfigured connector";
2939
@Autowired KafkaConnectService kafkaConnectService;
3040

3141
RestTemplate restTemplate;
@@ -73,4 +83,153 @@ public void getConnectorDetails_returnMap() throws JsonProcessingException {
7383
"conn1", "env", KafkaSupportedProtocol.PLAINTEXT, "CLID1"))
7484
.isNotEmpty();
7585
}
86+
87+
@Test
88+
public void createConnector_bad_request() throws Exception {
89+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
90+
91+
this.mockRestServiceServer
92+
.expect(requestTo("/env/connectors/conn1"))
93+
.andRespond(
94+
withBadRequest()
95+
.contentType(MediaType.APPLICATION_JSON)
96+
.body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR)));
97+
ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest);
98+
99+
assertThat(connectorResponse.isSuccess()).isFalse();
100+
assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR);
101+
}
102+
103+
private String getRestErrorResponse(String resp) throws JsonProcessingException {
104+
RestErrorResponse restErrorResponse = new RestErrorResponse();
105+
restErrorResponse.setErrorCode(400);
106+
restErrorResponse.setMessage(resp);
107+
return objectMapper.writeValueAsString(restErrorResponse);
108+
}
109+
110+
@Test
111+
public void createConnector_success() throws Exception {
112+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
113+
114+
this.mockRestServiceServer
115+
.expect(requestTo("/env/connectors/conn1"))
116+
.andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON));
117+
ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest);
118+
assertThat(connectorResponse.isSuccess()).isTrue();
119+
assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value);
120+
}
121+
122+
@Test
123+
public void createConnector_fail() throws Exception {
124+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
125+
126+
this.mockRestServiceServer
127+
.expect(requestTo("/env/connectors/conn1"))
128+
.andRespond(withRawStatus(207).contentType(MediaType.APPLICATION_JSON));
129+
ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest);
130+
assertThat(connectorResponse.isSuccess()).isFalse();
131+
assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.FAILURE.value);
132+
}
133+
134+
@Test
135+
public void updateConnector_success() {
136+
ClusterConnectorRequest connectorRequest = stubUpdateConnector();
137+
138+
this.mockRestServiceServer
139+
.expect(requestTo("/env/connectors/conn1/config"))
140+
.andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON));
141+
ApiResponse connectorResponse = kafkaConnectService.updateConnector(connectorRequest);
142+
assertThat(connectorResponse.isSuccess()).isTrue();
143+
assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value);
144+
}
145+
146+
@Test
147+
public void updateConnector_badRequest() throws JsonProcessingException {
148+
ClusterConnectorRequest connectorRequest = stubUpdateConnector();
149+
150+
this.mockRestServiceServer
151+
.expect(requestTo("/env/connectors/conn1/config"))
152+
.andRespond(
153+
withRawStatus(400)
154+
.contentType(MediaType.APPLICATION_JSON)
155+
.body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR)));
156+
ApiResponse connectorResponse = kafkaConnectService.updateConnector(connectorRequest);
157+
158+
assertThat(connectorResponse.isSuccess()).isFalse();
159+
assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR);
160+
}
161+
162+
@Test
163+
public void deleteConnector_badRequest() throws JsonProcessingException {
164+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
165+
166+
this.mockRestServiceServer
167+
.expect(requestTo("/env/connectors/conn1"))
168+
.andRespond(
169+
withRawStatus(400)
170+
.contentType(MediaType.APPLICATION_JSON)
171+
.body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR)));
172+
ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest);
173+
174+
assertThat(connectorResponse.isSuccess()).isFalse();
175+
assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR);
176+
}
177+
178+
@Test
179+
public void deleteConnector_badRequest_undetermined_response() throws JsonProcessingException {
180+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
181+
182+
this.mockRestServiceServer
183+
.expect(requestTo("/env/connectors/conn1"))
184+
.andRespond(
185+
withRawStatus(400)
186+
.contentType(MediaType.APPLICATION_JSON)
187+
.body(THIS_IS_A_MISCONFIGURED_CONNECTOR));
188+
ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest);
189+
190+
assertThat(connectorResponse.isSuccess()).isFalse();
191+
assertThat(connectorResponse.getMessage()).isEqualTo("Unable To Delete Connector on Cluster.");
192+
}
193+
194+
@Test
195+
public void deleteConnector_success() {
196+
ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector();
197+
198+
this.mockRestServiceServer
199+
.expect(requestTo("/env/connectors/conn1"))
200+
.andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON));
201+
ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest);
202+
assertThat(connectorResponse.isSuccess()).isTrue();
203+
assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value);
204+
}
205+
206+
private ClusterConnectorRequest stubCreateOrDeleteConnector() {
207+
when(getAdminClient.getRequestDetails(any(), eq(KafkaSupportedProtocol.PLAINTEXT)))
208+
.thenReturn(Pair.of("/env/connectors/conn1", restTemplate));
209+
when(getAdminClient.createHeaders(eq("1"), eq(KafkaClustersType.KAFKA_CONNECT)))
210+
.thenReturn(new HttpHeaders());
211+
ClusterConnectorRequest connectorRequest =
212+
ClusterConnectorRequest.builder()
213+
.connectorName("conn1")
214+
.clusterIdentification("1")
215+
.env("env")
216+
.protocol(KafkaSupportedProtocol.PLAINTEXT)
217+
.build();
218+
return connectorRequest;
219+
}
220+
221+
private ClusterConnectorRequest stubUpdateConnector() {
222+
when(getAdminClient.getRequestDetails(any(), eq(KafkaSupportedProtocol.PLAINTEXT)))
223+
.thenReturn(Pair.of("/env/connectors/conn1/config", restTemplate));
224+
when(getAdminClient.createHeaders(eq("1"), eq(KafkaClustersType.KAFKA_CONNECT)))
225+
.thenReturn(new HttpHeaders());
226+
ClusterConnectorRequest connectorRequest =
227+
ClusterConnectorRequest.builder()
228+
.connectorName("conn1")
229+
.clusterIdentification("1")
230+
.env("env")
231+
.protocol(KafkaSupportedProtocol.PLAINTEXT)
232+
.build();
233+
return connectorRequest;
234+
}
76235
}

‎core/src/main/java/io/aiven/klaw/controller/KafkaConnectController.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.aiven.klaw.controller;
22

33
import io.aiven.klaw.error.KlawException;
4+
import io.aiven.klaw.error.KlawRestException;
45
import io.aiven.klaw.model.ApiResponse;
56
import io.aiven.klaw.model.enums.Order;
67
import io.aiven.klaw.model.enums.RequestOperationType;
@@ -88,7 +89,7 @@ public ResponseEntity<ApiResponse> deleteConnectorRequests(
8889
value = "/execConnectorRequests",
8990
produces = {MediaType.APPLICATION_JSON_VALUE})
9091
public ResponseEntity<ApiResponse> approveTopicRequests(
91-
@RequestParam("connectorId") String connectorId) throws KlawException {
92+
@RequestParam("connectorId") String connectorId) throws KlawException, KlawRestException {
9293
return new ResponseEntity<>(
9394
kafkaConnectControllerService.approveConnectorRequests(connectorId), HttpStatus.OK);
9495
}

‎core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ public class KlawErrorMessages {
129129

130130
public static final String CLUSTER_API_ERR_121 = "Connection refused";
131131

132+
public static final String CLUSTER_API_ERR_122 = "doesn't match connector name in the URL";
133+
132134
// Env clusters tenants service
133135
public static final String ENV_CLUSTER_TNT_ERR_101 =
134136
"Failure. Please choose a different name. This environment name already exists.";

‎core/src/main/java/io/aiven/klaw/error/RestErrorResponse.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.aiven.klaw.error;
2+
3+
import com.fasterxml.jackson.annotation.JsonAlias;
4+
import lombok.Data;
5+
6+
@Data
7+
public class RestErrorResponse {
8+
9+
private String message;
10+
11+
@JsonAlias({"errorCode", "error_code"})
12+
private int errorCode;
13+
}

‎core/src/main/java/io/aiven/klaw/helpers/KwConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ public class KwConstants {
8181

8282
public static final String ORDER_OF_TOPIC_ENVS = "ORDER_OF_ENVS";
8383

84+
public static final String ORDER_OF_KAFKA_CONNECT_ENVS = "ORDER_OF_KAFKA_CONNECT_ENVS";
85+
8486
public static final int DAYS_EXPIRY_DEFAULT_TENANT = 365 * 10;
8587
public static final int DAYS_TRIAL_PERIOD = 7;
8688

0 commit comments

Comments
 (0)