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

Make parsing methods (Uri etc) more user friendly (or/and document exceptions etc) #1276

Open
johanandren opened this issue Jul 11, 2017 · 8 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module
Milestone

Comments

@johanandren
Copy link
Member

johanandren commented Jul 11, 2017

Sprung out of a discussion on gitter: https://gitter.im/akka/akka?at=5964806d4bcd78af569a46ff

Not sure about this one, but would it be better if for example the parse methods on Uri in addition to docs had the Scala @throws annotation detailing what exception they throw? (I don't think we have much methods throwing on the Akka core side of things so that's why I'm opening the issue here rather).

There might be more places where failure handling is done through exceptions, maybe the marshalling infra, that should have the same problem?

@johanandren johanandren added the discuss Tickets that need some discussion before proceeding label Jul 11, 2017
@johanandren
Copy link
Member Author

Another possible option which is more clear than inline scaladoc text is the scaladoc @throws tag.

@jlprat
Copy link
Member

jlprat commented Jul 11, 2017

What about offering safe methods that return a Try instead of throwing?

@johanandren
Copy link
Member Author

johanandren commented Jul 11, 2017

Yeah, also an option. That doesn't cover multiple heterogenous exceptions well though.

@bblfish
Copy link

bblfish commented Jul 11, 2017

I think it's exactly in these cases that having compiler support with throws clauses as Java has is useful - though I think one could have the best of both worlds by allowing organizations to decide whether they want to stick to that discipline and annotate a project somehow that they always declare exceptions.

Not that this is something that can be decided here, but perhaps it can be referenced by an appropriate issue in another relevant forum when and if it comes up.

From a developer point of view compiler support for declared exceptions is really helpful. Otherwise, the developer has to dig through the code...

@ktoso
Copy link
Member

ktoso commented Jul 12, 2017

Let's not go into language design and pros/cons of checked exceptions here ;-)

What I think is rather being asked for here is to take a second look at various parsing APIs and make them more user-friendly. They're mostly used internally where we value speed of getting a value out over the nice-to-use-nees, so exceptions won there. Perhaps adding versions try prefixed of methods could be good or an alternative way to make the parsing nicer to re-use by end users.

I'll change the title to be about that, it's also more actionable that way.

@ktoso ktoso changed the title Annotate methods throwing exceptions with @throws? Make parsing methods (URI etc) more user friendly (or/and document exceptions etc) Jul 12, 2017
@ktoso ktoso changed the title Make parsing methods (URI etc) more user friendly (or/and document exceptions etc) Make parsing methods (Uri etc) more user friendly (or/and document exceptions etc) Jul 12, 2017
@ktoso ktoso added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module and removed discuss Tickets that need some discussion before proceeding labels Jul 12, 2017
@ktoso ktoso added this to the 10.0.x milestone Jul 12, 2017
@jrudolph
Copy link
Member

I think it's quite undisputed to use Either/ Try / Validated for parsing functions that are expected to fail for some inputs.

I cannot really remember why that wasn't done here. One reason could be that part of those APIs was used internally and we wanted to avoid extra wrapping there but it might also just have been an oversight.

@bblfish
Copy link

bblfish commented Jul 12, 2017

It makes sense to also have a URL parsing method that throws exceptions for when this is used in code where a large number of URLs have to be parsed and most of them parse well.

@jrudolph
Copy link
Member

Yeah, and I realized that we have all kinds of usages like Uri("http://example.com") and even HttpRequest(uri = "http://example.com") that would also be affected. I guess part of the pain could be alleviated by statically checking URIs where that's possible but dynamic usages where URI strings are built dynamically can't be fixed as easily (but note how building URI strings is actually an anti-pattern itself).

@jrudolph jrudolph modified the milestones: 10.0.x, 10.2.x Aug 10, 2020
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 t:core Issues related to the akka-http-core module
Projects
None yet
Development

No branches or pull requests

5 participants