Skip to content

Commit eb708f4

Browse files
goshevgoshev
committed
DS-2384 - fix: malformed job payload errors to 400 and include jobId
Signed-off-by: Dimitar Goshev <dimitar.goshev@here.com>
1 parent a63a12b commit eb708f4

4 files changed

Lines changed: 150 additions & 7 deletions

File tree

‎xyz-jobs/xyz-job-service/src/main/java/com/here/xyz/jobs/service/JobApi.java‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static com.here.xyz.jobs.service.JobApiBase.ApiParam.Path.SPACE_ID;
2727
import static com.here.xyz.jobs.service.JobApiBase.ApiParam.getPathParam;
2828
import static com.here.xyz.jobs.steps.Step.InputSet.DEFAULT_SET_NAME;
29+
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
2930
import static io.netty.handler.codec.http.HttpResponseStatus.ACCEPTED;
3031
import static io.netty.handler.codec.http.HttpResponseStatus.CREATED;
3132
import static io.netty.handler.codec.http.HttpResponseStatus.OK;
@@ -50,6 +51,7 @@
5051
import io.vertx.core.Future;
5152
import io.vertx.ext.web.RoutingContext;
5253
import io.vertx.ext.web.openapi.router.RouterBuilder;
54+
import java.util.HashMap;
5355
import java.util.List;
5456
import java.util.Map;
5557
import java.util.Objects;
@@ -90,20 +92,47 @@ protected Future<Job> createNewJob(RoutingContext context, Job job) {
9092
return job.create().submit()
9193
.compose(v -> applyInputReferences(job))
9294
.map(res -> job)
93-
.recover(t -> {
94-
if (t instanceof CompilationError)
95-
return Future.failedFuture(new DetailedHttpException("E319002", t));
96-
if (t instanceof ValidationException)
97-
return Future.failedFuture(new DetailedHttpException("E319003", t));
98-
return Future.failedFuture(t);
99-
})
95+
.recover(t -> Future.failedFuture(normalizeJobCreationError(t, job.getId())))
10096
.onSuccess(res -> {
10197
sendResponse(context, CREATED.code(), res);
10298
logger.info(getMarker(context), "Job was created successfully: {}", job.serialize(true));
10399
})
104100
.onFailure(err -> sendErrorResponse(context, err));
105101
}
106102

103+
protected Throwable normalizeJobCreationError(Throwable error, String jobId) {
104+
final Throwable rootCause = rootCause(error);
105+
final Map<String, Object> errorDetails = Map.of("jobId", jobId);
106+
107+
if (error instanceof CompilationError)
108+
return new DetailedHttpException("E319002", placeholdersFor(error), errorDetails, error);
109+
110+
if (error instanceof ValidationException)
111+
return new DetailedHttpException("E319003", placeholdersFor(error), errorDetails, error);
112+
113+
if (rootCause instanceof ClassCastException)
114+
return new DetailedHttpException("E319003", placeholdersFor(rootCause), errorDetails, rootCause);
115+
116+
if (error instanceof IllegalArgumentException || error instanceof IllegalStateException)
117+
return new HttpException(BAD_REQUEST, error.getMessage(), errorDetails);
118+
119+
return error;
120+
}
121+
122+
private static Throwable rootCause(Throwable error) {
123+
Throwable current = error;
124+
while (current.getCause() != null && current.getCause() != current)
125+
current = current.getCause();
126+
return current;
127+
}
128+
129+
private static Map<String, String> placeholdersFor(Throwable error) {
130+
Map<String, String> placeholders = new HashMap<>();
131+
if (error != null && error.getMessage() != null)
132+
placeholders.put("cause", error.getMessage());
133+
return placeholders;
134+
}
135+
107136
protected Future<Void> applyInputReferences(Job job) {
108137
if (job.getInputs() == null)
109138
return Future.succeededFuture();

‎xyz-jobs/xyz-job-service/src/test/java/com/here/xyz/jobs/service/JobApiErrorNormalizationTest.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright (C) 2017-2026 HERE Europe B.V.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* License-Filename: LICENSE
18+
*/
19+
20+
package com.here.xyz.jobs.service;
21+
22+
import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST;
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
25+
import static org.junit.jupiter.api.Assertions.assertNotNull;
26+
27+
import com.here.xyz.jobs.steps.JobCompiler.CompilationError;
28+
import com.here.xyz.util.service.BaseHttpServerVerticle.ValidationException;
29+
import com.here.xyz.util.service.HttpException;
30+
import com.here.xyz.util.service.errors.DetailedHttpException;
31+
import com.here.xyz.util.service.errors.ErrorManager;
32+
import java.util.Map;
33+
import org.junit.jupiter.api.BeforeAll;
34+
import org.junit.jupiter.api.Test;
35+
36+
class JobApiErrorNormalizationTest {
37+
38+
private final JobApi api = new JobApi();
39+
40+
@BeforeAll
41+
static void loadErrorDefinitions() {
42+
try {
43+
ErrorManager.loadErrors("job-errors.json");
44+
}
45+
catch (RuntimeException e) {
46+
Throwable cause = e.getCause();
47+
if (!(cause instanceof IllegalStateException) || !cause.getMessage().contains("already registered"))
48+
throw e;
49+
}
50+
}
51+
52+
@Test
53+
void mapsCompilationErrorsToBadRequestWithJobId() {
54+
Throwable normalized = api.normalizeJobCreationError(new CompilationError("broken job"), "job-123");
55+
56+
DetailedHttpException detailed = assertInstanceOf(DetailedHttpException.class, normalized);
57+
assertEquals(BAD_REQUEST.code(), detailed.status.code());
58+
assertEquals("E319002", detailed.errorDefinition.getCode());
59+
assertNotNull(detailed.errorDetails);
60+
assertEquals("job-123", detailed.errorDetails.get("jobId"));
61+
}
62+
63+
@Test
64+
void mapsValidationErrorsToBadRequestWithJobId() {
65+
Throwable normalized = api.normalizeJobCreationError(new ValidationException("invalid payload"), "job-456");
66+
67+
DetailedHttpException detailed = assertInstanceOf(DetailedHttpException.class, normalized);
68+
assertEquals(BAD_REQUEST.code(), detailed.status.code());
69+
assertEquals("E319003", detailed.errorDefinition.getCode());
70+
assertNotNull(detailed.errorDetails);
71+
assertEquals("job-456", detailed.errorDetails.get("jobId"));
72+
}
73+
74+
@Test
75+
void mapsNestedClassCastErrorsToBadRequestWithJobId() {
76+
Throwable error = new RuntimeException(new ClassCastException("layerIds must be array"));
77+
78+
Throwable normalized = api.normalizeJobCreationError(error, "job-789");
79+
80+
DetailedHttpException detailed = assertInstanceOf(DetailedHttpException.class, normalized);
81+
assertEquals(BAD_REQUEST.code(), detailed.status.code());
82+
assertEquals("E319003", detailed.errorDefinition.getCode());
83+
assertNotNull(detailed.errorDetails);
84+
assertEquals("job-789", detailed.errorDetails.get("jobId"));
85+
}
86+
87+
@Test
88+
void mapsIllegalArgumentErrorsToBadRequestWithJobId() {
89+
Throwable normalized = api.normalizeJobCreationError(new IllegalArgumentException("bad argument"), "job-abc");
90+
91+
HttpException http = assertInstanceOf(HttpException.class, normalized);
92+
assertEquals(BAD_REQUEST.code(), http.status.code());
93+
assertEquals(Map.of("jobId", "job-abc"), http.errorDetails);
94+
}
95+
}
96+

‎xyz-util/src/main/java/com/here/xyz/util/service/HttpException.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,10 @@ public HttpException(HttpResponseStatus status, String errorText, Throwable caus
4545
this.status = status;
4646
this.errorDetails = null;
4747
}
48+
49+
public HttpException(HttpResponseStatus status, String errorText, Throwable cause, Map<String, Object> errorDetails) {
50+
super(errorText, cause);
51+
this.status = status;
52+
this.errorDetails = errorDetails;
53+
}
4854
}

‎xyz-util/src/main/java/com/here/xyz/util/service/errors/DetailedHttpException.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,16 @@ public DetailedHttpException(String errorCode, Map<String, String> placeholders,
6363
putAll(placeholders);
6464
}};
6565
}
66+
67+
public DetailedHttpException(String errorCode, Map<String, String> placeholders, Map<String, Object> errorDetails, Throwable cause) {
68+
super(HttpResponseStatus.valueOf(getErrorDefinition(errorCode).getStatus()), getErrorDefinition(errorCode).composeMessage(placeholders), cause,
69+
errorDetails);
70+
this.errorDefinition = getErrorDefinition(errorCode);
71+
this.placeholders = new HashMap<>() {{
72+
if (placeholders != null)
73+
putAll(placeholders);
74+
if (cause != null && cause.getMessage() != null)
75+
put("cause", cause.getMessage());
76+
}};
77+
}
6678
}

0 commit comments

Comments
 (0)