request(...).await(...) may block system messages

[dominiklohmann]

Note

Previously discussed in Gitter: https://app.gitter.im/#/room/#caf-community:gitter.im/$-flOmQATGnfq3hQiYSVnTLkSvgp4W3fFJHtWJ7Ikl2k

Using request(...).await(...) blocks exit and down messages, which—at least for exit messages—should probably be considered a bug. In our particular case, it has led to an actor lingering in a process until the request receiving actor shut down.

I think that no system message should be blocked by an awaited response. That is, however, a very hard-to-make breaking change, and I am not certain what the real-world impact of this change would be to existing library users.

[Neverlord]

Thanks for reporting!

That is, however, a very hard-to-make breaking change, and I am not certain what the real-world impact of this change would be to existing library users.

Let me think a bit about this. My first impulse is to actually tag this as bug and just fix it. If you would have asked me prior whether an exit message should be blocked by an await, I would've responded "no". Aside from exit messages, this is also suspending any processing for flows. Stalling data flows when using .await() is not exactly following the Principle of Least Surprise.

[dominiklohmann]

I totally agree that by the Principle of Least Surprise this should be considered a bug.

I went on a journey throughout our code base this morning, and I did not find a single place where this would break things, but much rather found two other places where we await during startup of an actor which would delay an immediate shutdowns.

[Neverlord]

I went on a journey throughout our code base this morning, and I did not find a single place where this would break things, but much rather found two other places where we await during startup of an actor which would delay an immediate shutdowns.

Thanks for going the extra mile. 🙂

I also can't think of a legit use case we would "break" by having the actor always respondingto system messages (which was my assumption how this is supposed to work from the beginning). So let's just call it a bug. I'll have a patch ready, filing as a PR shortly.