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.

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

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

Open
Fyro-Ing opened this issue Jun 11, 2024 · 5 comments
Open

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

Fyro-Ing opened this issue Jun 11, 2024 · 5 comments
Labels
Milestone

Comments

@Fyro-Ing
Copy link
Contributor

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 ..

@Fyro-Ing Fyro-Ing added the bug label Jun 11, 2024
@tsegismont tsegismont added this to the 4.5.9 milestone Jun 17, 2024
@tsegismont
Copy link
Contributor

cc @vietj

@vietj
Copy link
Contributor

vietj commented Jun 20, 2024

thanks I'll have a look

@vietj
Copy link
Contributor

vietj commented Jun 20, 2024

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
Copy link
Contributor

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

@vietj
Copy link
Contributor

vietj commented Jun 21, 2024

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.

@vietj vietj modified the milestones: 4.5.9, 4.5.10 Jul 17, 2024
@vietj vietj modified the milestones: 4.5.10, 4.5.11 Sep 4, 2024
@vietj vietj modified the milestones: 4.5.11, 4.5.12 Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants