-
Notifications
You must be signed in to change notification settings - Fork 596
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
UnexpectedConnectionClosureException cannot be caught since it's in private class #768
Comments
Would you submit a PR with the change? Thanks a lot.
…On Jan 14, 2017 11:03, "Sjoerd Mulder" ***@***.***> wrote:
Akka HTTP version 10.0.1
Somehow we have received a
Caused by: akka.http.impl.engine.client.OutgoingConnectionBlueprint$UnexpectedConnectionClosureException: The http server closed the connection unexpectedly before delivering responses for 1 outstanding requests
at akka.http.impl.engine.client.OutgoingConnectionBlueprint$$anonfun$apply$2.apply(OutgoingConnectionBlueprint.scala:127)
at akka.http.impl.engine.client.OutgoingConnectionBlueprint$$anonfun$apply$2.apply(OutgoingConnectionBlueprint.scala:127)
at akka.http.impl.util.One2OneBidiFlow$One2OneBidi$$anon$1$$anon$4.onUpstreamFinish(One2OneBidiFlow.scala:95)
at akka.stream.impl.fusing.GraphInterpreter.processEvent(GraphInterpreter.scala:732)
at akka.stream.impl.fusing.GraphInterpreter.execute(GraphInterpreter.scala:616)
But we should beable to .recover the future right? So i thought so let's
retry the request if that happens:
.recoverWith {
// The http server closed the connection unexpectedly before delivering responses
case e: UnexpectedConnectionClosureException if retries > 0 =>
logger.warn(s"Unexpected connection closure, retries left: $retries", e)
//Retry the request with 1 less retry
executeRequest(request, retries - 1)
But i'm not allowed to get to the exception since it's inside a private
class.
*Request*: make sure that all exception that can be thrown outside to the
system are part of the public API.
Error:(5, 37) object OutgoingConnectionBlueprint in package client cannot be accessed in package akka.http.impl.engine.client
import akka.http.impl.engine.client.OutgoingConnectionBlueprint.UnexpectedConnectionClosureException
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#768>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHYk2MRPEHS01IE3Zx2VF9hujJ1pczuks5rSJ1hgaJpZM4LjlUV>
.
|
Issue: 768 Make `UnexpectedConnectionClosureException` accessible for final users
We do not expose all internal exceptions. That doesn't mean that you cannot get at the exception message, it just means that you cannot rely on a particular exception type being thrown in a certain situation. This is standard practice for exception, i.e. the JDK doesn't offer all kinds of different On the other hand, in some cases, it can make sense to make the exception part of the API. But we need to carefully decide for each single one why we would expose them to the user and then add it to the documentation and add tests when such an exception would be thrown, etc. @sjoerdmulder why are you interested in that exception in particular and not in other ones? |
@jrudolph I'm interested in this one since i got this one thrown 🙂 I following your reasoning about This feels a bit brittle since the message might easily be changed in the future |
What is special about this particular failure condition? This error is thrown only in a very particular case which depends on the exact timing of the failure, very similar conditions might throw different kinds of errors. We need to identify the classes of errors users might be interested it, then we might add a public exception superclass for it. One class of failures could e.g. be "any kind of connection problem". |
@jrudolph Hello there, please excuse me for jumping right in the middle of the discussion. I totally agree with your reasoning regarding not exposing all internal exceptions. However, IOException and/or Socket or even ConnectException would be a better match for the parent of UnexpectedConnectionClosureException, imho. I discovered this issue while testing the resilience of our application. I start the test not being able to connect to the backend and I enable the connection (by starting a docker container) between retries. When the first connection is established, akka htt return an UnexpectedConnectionClosureException. Ideally, I would want to mark the Exception as a "connectivity issue" and allow the call to be retried. I hope this proves useful. Regards |
@midumitrescu, I agree having some hierarchy about the general class of failures would be nice. On the other hand, in general, the let-it-crash philosophy is also about not trying to add specific error handling code but treating the whole implementation as a black box and just retry regardless of the actual problem. But I also see (I also struggled with Akka's default before) that sometimes special-cased behavior actually is the better solution for a particular contexts.. |
This is still happening on |
We are currently having this issue (only coming up in test situations) and it would be helpful for us, at least, to be able to catch this exception. Although there are arguments for keeping it private (i.e. in production use you shouldn't be catching this at all) there are cases where you may want to catch this (in our case we are experiencing this now). It sounds like its a case of pragmatism vs being correct in design |
I guess the question is, in your test situation, why do you want to treat this specific exception specially and not just catch any Perhaps a generic public |
Well you can't catch this exception because it happens to be private (see #768). I also don't want to just catch any exception because sometimes this is indicative of a test failing
Well the thing is, if this is a bug in akka-http then it needs to be fixed |
In my case I had to catch it as a Throwable though: |
I just don't understand hours of discussions around this and at the end I am going to end up with a horrible client code that handles RuntimeExceptions for retry. End of the story! |
We are open to PRs but so far it was not a big enough problem to anyone to suggest one. |
@jrudolph possibly dumb question but what would be the issue on trivially making this exception non private? It seems like people are just manually catching it by checking |
It needs to be moved out of the Happy to discuss options if someone could make a concrete proposal in a PR. |
I see I did a PR with this a very long time ago: #979 We decided to close it because we didn't agree if it made sense or not.
Would that PR be a starting point for this? |
Akka HTTP version 10.0.1
Somehow we have received a
But we should beable to
.recover
the future right? So i thought so let's retry the request if that happens:But i'm not allowed to get to the exception since it's inside a private class.
Request: make sure that all exception that can be thrown outside to the system are part of the public API.
The text was updated successfully, but these errors were encountered: