Skip to content

Commit 7d8fc6b

Browse files
jknackjknack
committed
improve error reporting while parsing/injecting request params/headers fix #393 
1 parent fb9b520 commit 7d8fc6b

25 files changed

Lines changed: 388 additions & 207 deletions

‎coverage-report/src/test/java/org/jooby/issues/Issue177.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public void shouldSeeParamNameOnMvcRoute() throws Exception {
3030
request()
3131
.get("/issue177")
3232
.expect(400)
33-
.expect("Bad Request(400): Not found: x");
33+
.expect("Bad Request(400): Required parameter 'x' is not present");
3434
}
3535

3636
}

‎coverage-report/src/test/java/org/jooby/issues/Issue393.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.mvc.Path;
4+
import org.jooby.test.ServerFeature;
5+
import org.junit.Test;
6+
import org.slf4j.Logger;
7+
import org.slf4j.LoggerFactory;
8+
9+
public class Issue393 extends ServerFeature {
10+
11+
public static class Data {}
12+
13+
@Path("/r")
14+
public static class Resource {
15+
@Path("/param")
16+
public String param(final String value) {
17+
return value;
18+
}
19+
20+
@Path("/type")
21+
public Object type(final int value) {
22+
return value;
23+
}
24+
}
25+
26+
/** The logging system. */
27+
private final Logger log = LoggerFactory.getLogger(getClass());
28+
29+
{
30+
get("/param", req -> {
31+
return req.param("value").value();
32+
});
33+
34+
get("/no-parser", req -> {
35+
return req.param("value").to(Data.class);
36+
});
37+
38+
post("/body", req -> {
39+
return req.body().to(Data.class);
40+
});
41+
42+
get("/type", req -> {
43+
return req.param("value").intValue();
44+
});
45+
46+
use(Resource.class);
47+
48+
err((req, rsp, err) -> {
49+
log.error("issue 393 {}", req.route(), err);
50+
rsp.send(err.getMessage());
51+
});
52+
}
53+
54+
@Test
55+
public void missingParam() throws Exception {
56+
request()
57+
.get("/param")
58+
.expect(400)
59+
.expect("Bad Request(400): Required parameter 'value' is not present");
60+
61+
request()
62+
.get("/r/param")
63+
.expect(400)
64+
.expect("Bad Request(400): Required parameter 'value' is not present");
65+
}
66+
67+
@Test
68+
public void typeMismatch() throws Exception {
69+
request()
70+
.get("/type?value=x")
71+
.expect(400)
72+
.expect("Bad Request(400): Failed to parse parameter 'value' to 'int'");
73+
74+
request()
75+
.get("/r/type?value=x")
76+
.expect(400)
77+
.expect("Bad Request(400): Failed to parse parameter 'value' to 'int'");
78+
}
79+
80+
@Test
81+
public void body() throws Exception {
82+
request()
83+
.post("/body")
84+
.expect(415)
85+
.expect("Unsupported Media Type(415): Failed to parse body to 'org.jooby.issues.Issue393$Data'");
86+
}
87+
}

‎coverage-report/src/test/java/org/jooby/whoops/WhoopsTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import static org.easymock.EasyMock.expect;
44
import static org.easymock.EasyMock.expectLastCall;
55
import static org.junit.Assert.assertEquals;
6+
import static org.junit.Assert.assertTrue;
67

8+
import java.io.File;
79
import java.nio.file.Paths;
810
import java.util.Arrays;
911
import java.util.List;
@@ -48,7 +50,7 @@ public void shouldIgnoreMissingClass() {
4850
public void locationOf() {
4951
assertEquals("config-1.3.0.jar", Whoops.locationOf(Config.class));
5052

51-
assertEquals("target/classes", Whoops.locationOf(Whoops.class));
53+
assertTrue(new File(Whoops.locationOf(Whoops.class)).exists());
5254

5355
assertEquals("rt.jar", Whoops.locationOf(Object.class));
5456
}

‎jooby-hbv/src/main/java/org/jooby/hbv/Hbv.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import javax.validation.ConstraintValidatorFactory;
2929
import javax.validation.ConstraintViolationException;
30+
import javax.validation.ValidationException;
3031
import javax.validation.Validator;
3132

3233
import org.hibernate.validator.HibernateValidator;
@@ -40,6 +41,8 @@
4041
import com.google.inject.TypeLiteral;
4142
import com.google.inject.multibindings.Multibinder;
4243
import com.typesafe.config.Config;
44+
import com.typesafe.config.ConfigFactory;
45+
import com.typesafe.config.ConfigValueFactory;
4346

4447
/**
4548
* Bean validation via Hibernate Validator.
@@ -241,6 +244,13 @@ public void configure(final Env env, final Config config, final Binder binder) {
241244
.toInstance(new HbvParser(predicate));
242245
}
243246

247+
@Override
248+
public Config config() {
249+
return ConfigFactory.empty(Hbv.class.getName())
250+
.withValue("err." + ValidationException.class.getName(),
251+
ConfigValueFactory.fromAnyRef(400));
252+
}
253+
244254
static Predicate<TypeLiteral<?>> typeIs(final Class<?>[] classes) {
245255
return type -> {
246256
Class<?> it = type.getRawType();

‎jooby-hbv/src/test/java/org/jooby/hbv/HbvTest.java‎

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.easymock.EasyMock.expect;
44
import static org.easymock.EasyMock.isA;
5+
import static org.junit.Assert.assertEquals;
56

67
import java.util.function.BiConsumer;
78
import java.util.function.Consumer;
@@ -132,50 +133,56 @@ public void defaultsWithProperties() throws Exception {
132133
});
133134
}
134135

136+
@Test
137+
public void conf() throws Exception {
138+
assertEquals(ConfigFactory.empty().withValue("err.javax.validation.ValidationException",
139+
ConfigValueFactory.fromAnyRef(400)), new Hbv().config());
140+
}
141+
135142
@SuppressWarnings("unchecked")
136143
@Test
137144
public void defaultsWithConfigurer() throws Exception {
138145
new MockUnit(Env.class, Config.class, Binder.class, HibernateValidatorConfiguration.class,
139146
Consumer.class)
140-
.expect(hibernateValiConf)
141-
.expect(noproperties)
142-
.expect(validatorProvider)
143-
.expect(hvparser)
144-
.expect(unit -> {
145-
Consumer<HibernateValidatorConfiguration> configurer = unit.get(Consumer.class);
146-
147-
configurer.accept(unit.get(HibernateValidatorConfiguration.class));
148-
})
149-
.expect(onStop)
150-
.run(unit -> {
151-
new Hbv()
152-
.doWith(unit.get(Consumer.class))
153-
.configure(unit.get(Env.class), unit.get(Config.class), unit.get(Binder.class));
154-
});
147+
.expect(hibernateValiConf)
148+
.expect(noproperties)
149+
.expect(validatorProvider)
150+
.expect(hvparser)
151+
.expect(unit -> {
152+
Consumer<HibernateValidatorConfiguration> configurer = unit.get(Consumer.class);
153+
154+
configurer.accept(unit.get(HibernateValidatorConfiguration.class));
155+
})
156+
.expect(onStop)
157+
.run(unit -> {
158+
new Hbv()
159+
.doWith(unit.get(Consumer.class))
160+
.configure(unit.get(Env.class), unit.get(Config.class), unit.get(Binder.class));
161+
});
155162
}
156163

157164
@SuppressWarnings("unchecked")
158165
@Test
159166
public void defaultsWithFulleConfigurer() throws Exception {
160167
new MockUnit(Env.class, Config.class, Binder.class, HibernateValidatorConfiguration.class,
161168
BiConsumer.class)
162-
.expect(hibernateValiConf)
163-
.expect(noproperties)
164-
.expect(validatorProvider)
165-
.expect(hvparser)
166-
.expect(unit -> {
167-
BiConsumer<HibernateValidatorConfiguration, Config> configurer = unit
168-
.get(BiConsumer.class);
169-
170-
configurer.accept(unit.get(HibernateValidatorConfiguration.class),
171-
unit.get(Config.class));
172-
})
173-
.expect(onStop)
174-
.run(unit -> {
175-
new Hbv()
176-
.doWith(unit.get(BiConsumer.class))
177-
.configure(unit.get(Env.class), unit.get(Config.class), unit.get(Binder.class));
178-
});
169+
.expect(hibernateValiConf)
170+
.expect(noproperties)
171+
.expect(validatorProvider)
172+
.expect(hvparser)
173+
.expect(unit -> {
174+
BiConsumer<HibernateValidatorConfiguration, Config> configurer = unit
175+
.get(BiConsumer.class);
176+
177+
configurer.accept(unit.get(HibernateValidatorConfiguration.class),
178+
unit.get(Config.class));
179+
})
180+
.expect(onStop)
181+
.run(unit -> {
182+
new Hbv()
183+
.doWith(unit.get(BiConsumer.class))
184+
.configure(unit.get(Env.class), unit.get(Config.class), unit.get(Binder.class));
185+
});
179186
}
180187

181188
}

‎jooby/src/main/java/org/jooby/Parser.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ interface Callback<T> {
101101
*/
102102
interface ParamReference<T> extends Iterable<T> {
103103

104+
/**
105+
* @return Descriptive type: parameter, header, cookie, etc...
106+
*/
107+
String type();
108+
104109
/**
105110
* @return Parameter name.
106111
*/

‎jooby/src/main/java/org/jooby/internal/HttpHandlerImpl.java‎

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ public boolean equals(final Object obj) {
173173

174174
private final Map<String, Renderer> rendererMap;
175175

176+
private StatusCodeProvider sc;
177+
176178
@Inject
177179
public HttpHandlerImpl(final Injector injector,
178180
final RequestScope requestScope,
@@ -182,6 +184,7 @@ public HttpHandlerImpl(final Injector injector,
182184
final ParserExecutor parserExecutor,
183185
final Set<Renderer> renderers,
184186
final Set<Err.Handler> err,
187+
final StatusCodeProvider sc,
185188
final Charset charset,
186189
final List<Locale> locale) {
187190
this.injector = requireNonNull(injector, "An injector is required.");
@@ -190,6 +193,7 @@ public HttpHandlerImpl(final Injector injector,
190193
this.hasSockets = socketDefs.size() > 0;
191194
this.applicationPath = normalizeURI(requireNonNull(path, "An application.path is required."));
192195
this.err = requireNonNull(err, "An err handler is required.");
196+
this.sc = sc;
193197
this.config = injector.getInstance(Config.class);
194198
_method = Strings.emptyToNull(this.config.getString("server.http.Method").trim());
195199
this.port = config.getInt("application.port");
@@ -348,7 +352,7 @@ private void handleErr(final RequestImpl req, final ResponseImpl rsp, final Thro
348352
rsp.reset();
349353

350354
// execution failed, find status code
351-
Status status = statusCode(ex);
355+
Status status = sc.apply(ex);
352356

353357
rsp.header("Cache-Control", NO_CACHE);
354358
rsp.status(status);
@@ -419,34 +423,6 @@ private static Optional<WebSocket> findSockets(final Set<WebSocket.Definition> s
419423
return Optional.empty();
420424
}
421425

422-
private Status statusCode(final Throwable ex) {
423-
if (ex instanceof Err) {
424-
return Status.valueOf(((Err) ex).statusCode());
425-
}
426-
/**
427-
* usually a class name, except for inner classes where '$' is replaced it by '.'
428-
*/
429-
Function<Class<?>, String> name = type -> Optional.ofNullable(type.getDeclaringClass())
430-
.map(dc -> new StringBuilder(dc.getName())
431-
.append('.')
432-
.append(type.getSimpleName())
433-
.toString())
434-
.orElse(type.getName());
435-
436-
Config err = config.getConfig("err");
437-
int status = -1;
438-
Class<?> type = ex.getClass();
439-
while (type != Throwable.class && status == -1) {
440-
String classname = name.apply(type);
441-
if (err.hasPath(classname)) {
442-
status = err.getInt(classname);
443-
} else {
444-
type = type.getSuperclass();
445-
}
446-
}
447-
return status == -1 ? Status.SERVER_ERROR : Status.valueOf(status);
448-
}
449-
450426
private static Err handle405(final Set<Route.Definition> routeDefs, final String method,
451427
final String uri, final MediaType type, final List<MediaType> accept) {
452428

0 commit comments

Comments
 (0)