vert-x3 / vertx-web

HTTP web applications for Vert.x
Apache License 2.0
1.11k stars 532 forks source link

New web client expectation, result type differ from error type #2624

Open Fyro-Ing opened 2 months ago

Fyro-Ing commented 2 months ago

Switching to new web client expectation ( #2607 ) occurre bug when body codec response is set.

Response is decoded prior to check expecting and result to a DecodedException

Version Vertx 4.5.8

Do you have a reproducer?

@Test
public void newExpect() throws Exception {
  UUID uuid = UUID.randomUUID();
  int statusCode = 400;

  Expectation<HttpResponseHead> expectation = HttpResponseExpectation.SC_SUCCESS
    .wrappingFailure((head, err) -> new CustomException(uuid, String.valueOf(head.statusCode())));

  server.requestHandler(request -> request.response()
    .setStatusCode(statusCode)
    .end("{}"));

  startServer();
  HttpRequest<Integer> request = webClient
    .get("/test")
    .as(json(Integer.class));

  request.send().expecting(expectation).onComplete(ar -> {
    Throwable cause = ar.cause();
    assertThat(cause, instanceOf(CustomException.class));
    testComplete();
  });
  await();
}
@Test
public void oldExpect() throws Exception {
  UUID uuid = UUID.randomUUID();
  int statusCode = 400;

  server.requestHandler(request -> request.response()
    .setStatusCode(statusCode)
    .end("{}"));

  startServer();
  HttpRequest<Integer> request = webClient
    .get("/test")
    .as(json(Integer.class));

  final ResponsePredicate predicate = ResponsePredicate.create(ResponsePredicate.SC_SUCCESS, result ->
    new CustomException(uuid, String.valueOf(result.response().statusCode()))
  );

  request.expect(predicate).send().onComplete(ar -> {
    Throwable cause = ar.cause();
    assertThat(cause, instanceOf(CustomException.class));
    testComplete();
  });
  await();
}

same with JsonObject vs JsonArray, etc ..

tsegismont commented 2 months ago

cc @vietj

vietj commented 2 months ago

thanks I'll have a look

vietj commented 2 months ago

I see the behaviour is different for this case, I am thinking that the decoding of the body should be applied only when responses are 2xx that would fix this issue

tsegismont commented 2 months ago

The body may contain useful information (e.g. problem ID) in case of an error response. How can we get it?

vietj commented 2 months ago

more thoughts about this: since expecting is a construct that operates at the future level and not the http request/response api level, there is no way we can have the decoding happening after the expectation.

Instead the Future.map operator should be used, so the code would actually be like:

webClient
    .get("/test")
    .expecting(expectation)
    .map(response -> response.bodyAsJson(Integer.class));

Therefore we might have to deprecate the the as on HttpRequest to discourage using it, if that can be replaced by the use of map.