UnexpectedConnectionClosureException cannot be caught since it's in private class

[sjoerdmulder]

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.

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

Request: make sure that all exception that can be thrown outside to the system are part of the public API.

[ktoso]

Would you submit a PR with the change? Thanks a lot.

On Jan 14, 2017 11:03, "Sjoerd Mulder" [email protected] 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 https://github.com/akka/akka-http/issues/768, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHYk2MRPEHS01IE3Zx2VF9hujJ1pczuks5rSJ1hgaJpZM4LjlUV .

[jrudolph]

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 IOException for every single potential failure condition. It's hard enough to keep the API compatible in the happy cases, we don't want to be bound to be exception-compatible as well.

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?

[sjoerdmulder]

@jrudolph I'm interested in this one since i got this one thrown :slightly_smiling_face:

I following your reasoning about IOException but currently the superclass of this exception is just RuntimeException which makes the pattern match look like: case e: RuntimeException if e.getMessage.contains("The http server closed the connection unexpectedly")

This feels a bit brittle since the message might easily be changed in the future

[jrudolph]

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".

[midumitrescu]

@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. For my usecase, I need to execute a rest call to a backend.

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

[jrudolph]

@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..

[vertexclique]

This is still happening on 10.0.9

[mdedetrich]

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

[raboof]

I guess the question is, in your test situation, why do you want to treat this specific exception specially and not just catch any Exception?

Perhaps a generic public ConnectionException (that covers not just this specific problem but a wider range of possible issues) would be both correct in design and solve your 'pragmatic' problem?

[mdedetrich]

I guess the question is, in your test situation, why do you want to treat this specific exception specially and not just catch any Exception?

Well you can't catch this exception because it happens to be private (see https://github.com/akka/akka-http/issues/768). I also don't want to just catch any exception because sometimes this is indicative of a test failing

Perhaps a generic public ConnectionException (that covers not just this specific problem but a wider range of possible issues) would be both correct in design and solve your 'pragmatic' problem?

Well the thing is, if this is a bug in akka-http then it needs to be fixed

[dirkdior]

@jrudolph I'm interested in this one since i got this one thrown

I following your reasoning about IOException but currently the superclass of this exception is just RuntimeException which makes the pattern match look like: case e: RuntimeException if e.getMessage.contains("The http server closed the connection unexpectedly")

This feels a bit brittle since the message might easily be changed in the future

In my case I had to catch it as a Throwable though: case error: Throwable if error.getMessage.contains("The http server closed the connection unexpectedly") => but thanks, that helped.

[ergunbaris]

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!

[jrudolph]

We are open to PRs but so far it was not a big enough problem to anyone to suggest one.

[mdedetrich]

@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 .getMessage anyways which is arguably indicative it shouldn't have been private in the first place.

[jrudolph]

It needs to be moved out of the impl package to a final place. As said before, it would be good to have a sensible hierarchy of exceptions that we can safely extend later on. We can also start with an API marked as @ApiMayChange so we can still change it later if we think it's necessary.

Happy to discuss options if someone could make a concrete proposal in a PR.

[jlprat]

I see I did a PR with this a very long time ago: https://github.com/akka/akka-http/pull/979 We decided to close it because we didn't agree if it made sense or not.

We can also start with an API marked as @ApiMayChange so we can still change it later if we think it's necessary.

Would that PR be a starting point for this?