rho Return 400 if required query parameter is missing

If method and path in request match some route but don't contain mandatory query parameters I would expect a 400 error response rather then attempts to match subsequent routes. Otherwise, it's hard to do error handling properly. This gist gives an example of what I mean: https://gist.github.com/paul-lysak/c3305c5bb0771fd97e9f4e7041dabfcb It's possible to emulate such behavior by adding one more route with the same path and without query parameters, but it comes at a cost:

it won't be able to tell which query parameters are missing if there are multiple query parameters
generated Swagger spec won't contain any definitions for query parameters - it describes only the "fallback" route

This is somewhat similar to https://github.com/http4s/rho/issues/55.

Sep 28 '18 15:09 paul-lysak

I'd expect exactly this behavior: in the first route we stated that service needs a barId param, then client didn't provide it and that's why URL was not matched. I think if /foo/bar needs to return 400 then we have to handle it explicitly: make barId optional and check if it is provided/valid or return 400 otherwise.

Sep 30 '18 18:09 chuwy

@chuwy the swagger spec would be incorrect then which undermines the whole point of using the Rho - a required parameter will be indicated as optional there.

Oct 01 '18 06:10 paul-lysak

I think we need to decide here if we want to:

Match it and return 400 (with optional parameter)
Not match it (and have required parameter)

Both options look valid to me when they're explicitly stated by developer. But maybe this is some kind of personal preference, I'd like to hear other people's opinion.

Oct 01 '18 08:10 chuwy

I suspect we should only be matching on the path and not on the query parameters, and letting the route fail if required parameters aren't provided. Unfortunately we can't (currently) prevent users from accidentally writing 2 routes with the same path but different query strings matchers, so users would only ever see the first fail.

I really wonder what the original design choice was here and why?

Any thoughts @bryce-anderson or @rossabaker?

Oct 07 '18 13:10 zarthross