Helping Mutiny users not run into dumb user issues

[Stephane Epardaud]

Hi,

I just realised that this code passed all the tests, but only because it wasn't actually invoked:

Panache.getSession().map(session -> session.withTransaction(tx -> {

  // run test

}).await().indefinitely();

Can you spot it?

I'm not sure how frequent it is to mess something up with Mutiny, this way or other ways, but perhaps we could make it easier for users to notice these traps somehow?

Perhaps in Quarkus test mode we could inspect the returned value from map operations and if it's a Uni print a warning? This doesn't seem very useful because people tend to ignore warnings.

Another option would be to declare a map operation explicitely for returning a Uni, and deprecate it with a warning that this is probably not what the user wanted to do? I'm not even sure erasure rules would allow this.

I'm not really sold on the idea, and I don't know if there are other common traps, but if there are, we could try to come up with ways to nudge users into the right direction. Perhaps it's not possible, or not worth it. But definitely worth discussing :)

--

Stéphane Épardaud

[clement escoffier]

I have to admit that I got caught by this a few times. 

The problem is that we are actually receiving a Uni<Uni<Void>>, so it’s all fine… except that it’s not what we want. 

Maybe we can report a warning somehow. We can verify that the mapper function does not return a `Uni`. Note that there are case where it’s what you want. There are rare, but I used that a few times.  

Clement

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKU9E9tQH-J8qzZCxfTYmC3FOKedbF3G%3DyOS1Ld_t4idDJtwmg%40mail.gmail.com.

[Stephane Epardaud]

Yeah, I figured it must be pretty rare to want `map` to return a `Uni`. It's interesting you also got bitten by this. I wonder what other operations can be tricky.

Perhaps the thing we should be looking for is a `Uni` that is never chained or subscribed to? Or perhaps even more specifically a lazy `Uni` never subscribed to?

--

Stéphane Épardaud

[Ladislav Thon]

The very existence of something like dev mode enables all kinds of expensive checks that would be insane to do in production.

What Stef suggests is a perfect example. Couldn't we remember all Unis (and Multis?) that are created, and periodically check if that remembered set contains something that noone subscribed to? We should even be able to capture the stack trace at the moment of creation, to point users to exactly where the Uni was created. (One obvious disadvantage is that this would also store all intermediate operators. But operators know they are operators, so they could skip themselves, maybe?)

Biggest problem I see with this is, how to present the finding to users? An error message in the console and/or dev UI? Remembering that an error was detected asynchronously and throwing it at the user in their first subsequent HTTP request? :-)

LT

st 9. 6. 2021 v 17:02 odesílatel Stephane Epardaud <stephane...@gmail.com> napsal:

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKU9E9v_thKTfYV0S4fjJ%3DabcPrznKu_6p1JZLzq-Y2KnnHwvg%40mail.gmail.com.

[clement escoffier]

We can totally capture Uni/Multi that get created. We have hooks for this. We can also check if they got a subscriber. 

About the reporting, I would dump random characters (aka stack traces) in the console. That might be intrusive, but at least the user would notice. Of course, we would also add a flag to disable that.

But, the question is how long should we wait? Typically, let's imagine I've a @Channel injected, it's a Multi, and it's perfectly fine that no one subscribes to it if no one needs the items. 

Clement

[Stuart Douglas]

So I have done something similar in Undertow 2.x to detect buffer leaks. If buffers were allocated during the test but not freed my test extension would track this and fail the test.

I think we can do something similar here, as each test gives a defined lifecycle for the Uni. I think we should only do this for 'user code' Uni/Multi though, as I know we have some lazy Uni instances that may not be subscribed to, and it is not a problem. E.g. if lazy auth is enabled the user is represented by Uni<SecurityIdentity> and if nothing actually asks for a user it is not resolved.

Stuart

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKW6fidf5%3Dy_1rjOhwRdks1haykqHnm-j-arV%3DuixFm00mWvZA%40mail.gmail.com.

[Ladislav Thon]

I was thinking if we track the origin of Uni creation (which we would like to show users, so that they know where to look), we could filter out some known origins (such as io.quarkus.* and io.smallrye.*). Which wouldn't help Stef then -- sorry :-)

LT

čt 10. 6. 2021 v 4:14 odesílatel Stuart Douglas <sdou...@redhat.com> napsal: