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

Document that an exception thrown in a Path param unmarshaller is a rejection #18

Closed
akka-ci opened this issue Sep 8, 2016 · 9 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on small t:docs Issues related to the documentation
Milestone

Comments

@akka-ci
Copy link

akka-ci commented Sep 8, 2016

Issue by ktoso
Sunday Jul 10, 2016 at 14:01 GMT
Originally opened as akka/akka#20929


... and the next path will be attempted to be resolved.

This is different than exceptions inside of a route already (which would render as 500 usually then)

@akka-ci akka-ci added this to the 2.4.x milestone Sep 8, 2016
@akka-ci akka-ci added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on small t:docs Issues related to the documentation t:http labels Sep 8, 2016
@ktoso ktoso removed the t:http label Sep 12, 2016
@ktoso ktoso removed this from the 2.4.x milestone Sep 12, 2016
@ilke-zilci
Copy link
Contributor

Hey, I'd like to take this one, is anyone working on it already?

@ktoso
Copy link
Member

ktoso commented Jul 13, 2017

No, seems no one has picked it up yet, thanks for looking into it! :)
(Feel free to ask if not sure about things with it etc)

@jrudolph
Copy link
Member

jrudolph commented Jul 13, 2017

I agree, might be a good starting contribution!

However, I find the ticket description a bit unclear. I think it's about using parameter("age".as(xyzUnmarshaller)) and this xyzUnmarshaller throws an exception? Do you remember if that was the case here, @ktoso?

In that case, this code here seems to convert the exception into a rejection: https://github.com/akka/akka-http/blob/master/akka-http/src/main/scala/akka/http/scaladsl/server/directives/ParameterDirectives.scala#L115

It should probably be documented in the documentation of the parameters directive here: https://github.com/akka/akka-http/blob/master/docs/src/main/paradox/scala/http/routing-dsl/directives/parameter-directives/parameters.md

@ktoso
Copy link
Member

ktoso commented Jul 13, 2017

Yes, it was about that; AFAIR someone expected "well it's an exception so should be an error page!", but indeed there we interpret it as "well, unable to extract, let's try another one", so as a rejection :)

@jlprat
Copy link
Member

jlprat commented Jul 13, 2017

Good to see you here @ilke-zilci!

@ilke-zilci
Copy link
Contributor

@jlprat
Copy link
Member

jlprat commented Jul 14, 2017

As @jrudolph mentioned, I would document it next to the Path Parameter documentation pages (https://github.com/akka/akka-http/blob/master/docs/src/main/paradox/scala/http/routing-dsl/directives/parameter-directives/parameters.md) and then link to the rejection doc page as a "see for further info".

And don't forget to document it for both Scala and Java docs :)

@ilke-zilci
Copy link
Contributor

Ok thanks. And sorry missed that line on the first read.

@jrudolph
Copy link
Member

Fixed in #1285.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on small t:docs Issues related to the documentation
Projects
None yet
Development

No branches or pull requests

5 participants