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

fix #23416 instantiate exceptions at right time #23417

Merged
merged 1 commit into from Aug 9, 2017
Merged

fix #23416 instantiate exceptions at right time #23417

merged 1 commit into from Aug 9, 2017

Conversation

francisdb
Copy link
Contributor

fix #23416 instantiate exceptions at right time

@akka-ci
Copy link

akka-ci commented Jul 25, 2017

Can one of the repo owners verify this patch?

@ktoso
Copy link
Member

ktoso commented Jul 25, 2017

OK TO TEST

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR is currently being validated by Jenkins labels Jul 25, 2017
@akka-ci
Copy link

akka-ci commented Jul 25, 2017

Test PASSed.

@ktoso
Copy link
Member

ktoso commented Jul 25, 2017

I think this was intentional, since the stacktrace won't help you as much as you'd think it would...
NoStacktrace would address the perf issue - but is there really one? How often are these things thrown really

@francisdb
Copy link
Contributor Author

francisdb commented Jul 25, 2017

I'm having this being thrown and am trying to debug what is going on, since the cause might be in the implementation of the reading/writing done by a different lib the stack trace would be more than welcome. Currently the trace points to the location where .run() is called which is very confusing.

java.io.IOException: Reactive stream is terminated, no writes are possible
	at akka.stream.impl.io.OutputStreamAdapter.<init>(OutputStreamSourceStage.scala:164)
	at akka.stream.impl.io.OutputStreamSourceStage.createLogicAndMaterializedValue(OutputStreamSourceStage.scala:151)
	at akka.stream.impl.GraphStageIsland.materializeAtomic(PhasedFusingActorMaterializer.scala:627)
	at akka.stream.impl.PhasedFusingActorMaterializer.materialize(PhasedFusingActorMaterializer.scala:458)
	at akka.stream.impl.PhasedFusingActorMaterializer.materialize(PhasedFusingActorMaterializer.scala:420)
	at akka.stream.impl.PhasedFusingActorMaterializer.materialize(PhasedFusingActorMaterializer.scala:415)
	at akka.stream.scaladsl.RunnableGraph.run(Flow.scala:439)
	at my.project.ImportExport$.readZip(ImportExport.scala:66)
...

where the line in my code is this

val is = source.toMat(StreamConverters.asInputStream(5.seconds))(Keep.right).run()
// the reading of the InputStream is handled further down in the code using a Future

The error is probably caused by the library I am using that writes to the stream and closes/flushes multiple times.

Also this is the only place in Akka streams where I see this pattern of pre-creating the exception. Sure this is not an accidental mistake?

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the stack trace will be useful, but I also think caching the exception was a premature optimisation, so LGTM

@francisdb
Copy link
Contributor Author

added some more proof of the usefulness of this patch in the ticket:
#23416 (comment)

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@patriknw patriknw merged commit 3ba093d into akka:master Aug 9, 2017
@francisdb francisdb deleted the issue23416 branch August 13, 2017 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid stack trace because reused exception code is defined as val
5 participants