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

Spring Webflux 406 errors when ContentType of Method differs from ExceptionHandler #13635

Closed
pluttrell opened this issue Jul 2, 2018 · 9 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@pluttrell
Copy link

I'm using Spring Webflux and seeing 406 errors when both of the following occur:

  1. Exceptions are returned from the the Reactive stream instead of the controller method itself.
  2. When the controller method produces a different ContentType then the ExceptionHandler.

This is reproducible using SpringBoot v2.0.3 and v2.1.0-BUILD-SNAPSHOT (as of today).

@RestController
@RequiredArgsConstructor
class BinaryTestController {

  private final AccessKeyValidationService accessKeyValidationService;

  @GetMapping(path = "/binary-test", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE)
  Mono<byte[]> test(@RequestParam("apiKey") String apiKey, @RequestParam(name = "failFirst", required = false) boolean failFirst) {

    if (failFirst) { throw new IllegalArgumentException();}

    return Mono.just(apiKey)
        .filterWhen(accessKeyValidationService::isValid)
        .switchIfEmpty(Mono.error(new IllegalAccessException()))
        .then(Mono.just("test".getBytes(StandardCharsets.UTF_8)));
  }

  @ExceptionHandler(IllegalArgumentException.class)
  ResponseEntity<Mono<BadResponse>> handleIllegalArgumentException() {
    HttpHeaders headers = new HttpHeaders();
    headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
    return new ResponseEntity<>(Mono.just(new BadResponse("better luck next time")), headers, HttpStatus.BAD_REQUEST);
  }

  @ExceptionHandler(IllegalAccessException.class)
  ResponseEntity<Mono<BadResponse>> handleIllegalAccessException() {
    HttpHeaders headers = new HttpHeaders();
    headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
    return new ResponseEntity<>(Mono.just(new BadResponse("access denied")), headers, HttpStatus.FORBIDDEN);
  }

  @Data
  class BadResponse {
    private final String developerMessage;
  }

}

The AccessKeyValidationService does a blocking lookup, so naturally needs to return a Mono and thus needs to be part of the Mono returned from the controller:

@Service
class AccessKeyValidationService {

  Mono<Boolean> isValid(String accessKey) {
    return Mono.defer(() -> Mono.just(blockingLookup(accessKey)))
        .subscribeOn(Schedulers.elastic());
  }

  private Boolean blockingLookup(String accessKey) {
    //Some blocking lookup...
    return accessKey.equals("good");
  }
}

A valid apiKey, results in proper binary output:

$ http localhost:8080/binary-test apiKey==good
HTTP/1.1 200 OK
Content-Length: 4
Content-Type: application/octet-stream

test

When I throw an exception before the Reactive Stream, I get the intended JSON output:

$ http localhost:8080/binary-test apiKey==good failFirst==true
HTTP/1.1 400 Bad Request
Content-Length: 44
Content-Type: application/json;charset=UTF-8

{
    "developerMessage": "better luck next time"
}

But when I trigger the steam to error with an IllegalAccessException, I get a 406:

$ http localhost:8080/binary-test apiKey==bad
HTTP/1.1 406 Not Acceptable
Content-Length: 157
Content-Type: application/json;charset=UTF-8

{
    "error": "Not Acceptable",
    "message": "Could not find acceptable representation",
    "path": "/binary-test",
    "status": 406,
    "timestamp": "2018-06-30T05:23:09.820+0000"
}

If I put a breakpoint in the handleIllegalAccessException method, it stops as expected.

If I switch the return type for the controller method to JSON it works as expected.

Is there something else I need to do to switch the response type when erroring out the returned Mono? Or some other way to resolve it?

Here's a repo with a failing unit test reproducing the problem: https://github.com/pluttrell/spring-webflux-binary-response-exception-handling-example

@pluttrell
Copy link
Author

@bclozel As requested on Gitter, here's the issue.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 2, 2018
@pluttrell pluttrell changed the title Spring Webflux 406 errors when ContentType of Method differs from ExceptionHandler Spring Webflux 406 errors when ContentType of Method differs from ExceptionHandler Jul 2, 2018
@bclozel bclozel self-assigned this Jul 2, 2018
@pluttrell
Copy link
Author

@bclozel Wondering if you might have any thoughts on this issue. Thanks in advance.

@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 24, 2018
@bclozel
Copy link
Member

bclozel commented Jul 24, 2018

Thanks for the report @pluttrell !
This is actually an issue in Spring Framework - I've created SPR-17082 to track it.

I'm about to solve it and the fix will ship with Spring Framework 5.0.8 and 5.1.0.RC1, so Spring Boot 2.0.4 and 2.1.0.M1.

I'm marking it as invalid because technically the issue is not in Spring Boot, but it doesn't mean your contribution is not valued.

Thanks!

@bclozel bclozel closed this as completed Jul 24, 2018
@pluttrell
Copy link
Author

@bclozel When upgrading to 2.1.0.M4, this issue was resolved. However when moving to 2.1.0.RC1, the issue came back. I've reported the potential regression here: https://jira.spring.io/browse/SPR-17415

@bidadh
Copy link

bidadh commented Jan 4, 2019

@bclozel I have exactly same issue with spring-boot 2.1.1.RELEASE. an I missing something ?

@bclozel
Copy link
Member

bclozel commented Jan 4, 2019

@bidadh I don't know - could you create a new issue with a sample application that demonstrates the problem?

@bidadh
Copy link

bidadh commented Jan 4, 2019

@bclozel yes I can. I thought because this is exactly the same issue maybe it's better to add a comment.

I can also add Controller and ExceptionHandler code here

which one easier for you ?

@bclozel
Copy link
Member

bclozel commented Jan 4, 2019

A new issue with a complete repro app (for example in a github repo of yours). Sometimes those are tied to dependency management or other issues so a complete repro makes things easier and faster to fix.

@bidadh
Copy link

bidadh commented Jan 12, 2019

Hi @bclozel

I've created this issue

and here is the repository

Could you please have a look ?

Thanks,
Arthur

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants