IQ result routes

[wichert]

This is an attempt to fix #78. The approach I am taking is to introduce an API like this to the router:

router.NewIqResultRoute(context.Background(), "9128171").HandlerFunc(func (s Sender, p stanza.Packet) {
    // Process IQ result 
})

The context is there to support cancellation and timeouts. They have become a pretty standard feature in golang webservers, and work well here. With a timeout the example becomes:

router.NewIqResultRoute(context.WithTimeout(time.Second * 30, context.Background()), "9128171")
  .HandlerFunc(func (s Sender, p stanza.Packet) {
      // Process IQ result 
  })
  .TimeoutHandlerFunc(func (err error) {
      // Handle a timeout
  })

To make this a little easier to use I am thinking of adding a method to Client to combine sending an IQ and adding a result route. Something like:

func (c Client) SendIq(iq stanza.Packet, handler HandlerFunc) (*IqResultRoute, error) {
    if err := c.Send(iq); err != nil {
        return nil, err
    }
    return c.router.NewIqResultRoute(context.Background(), iq.Attrs.id).HandlerFunc(handler), nil
}

@mremond Does this look sensible to you?

Remaining TODO items:

• Add router tests
• Expire old entries in iqResultRoutes
• If a result route is replaced cancel its context (if possible?)

[wichert]

@mremond This is ready for review now.

[wichert]

I'm happy with the API, but it turns out to have one major flaw: it is leaking contexts. Back to the drawing board again!

[wichert]

I have reworked the API to use a result channel, which also feels a lot more idiomatic. Usage is pretty simple.

iq := stanza.NewIQ(stanza.Attrs{Type: stanza.IQTypeGet, To: "…"})
result, err := client.SendIQ(context.Background(), iq)
reply <- result
fmt.Printf("We received an IQ result: %v", reply)

If you want to do full error handling and add a 30 second timeout (which I would highly recommend) the code becomes a bit more verbose, but it does feel like pretty standard Go to me:

iq := stanza.NewIQ(stanza.Attrs{Type: stanza.IQTypeGet, To: "…"})
ctx, _ := context.WithTimeout(context.Background(), time.Second * 30)
def ctx.Cancel()
result, err := client.SendIQ(ctx, iq)
if err != nil {
        log.Fatalf*("Error sending IQ message: %v", err)
}
select {
case iq := <- result:
        fmt.Printf("We received a result: %v", iq)
case <- ctx.Done():
        fmt.Printf("No result received in 30 seconds ☹️");
})

One thing to note is that there was a possible deadlock if a stanza handler wanted to send an IQ itself: in that case Router.route would block while sending a message to the IQResultRoute's result channel. This is especially obvious in the router tests, where the route would try to do that before the test started receiving on the channel. I fixed that by always invoking Router.route as a go-routine. I was thinking about doing that anyway to allow processing of multiple stanzas in parallel.

The tests pass with -race enabled.

[wichert]

I changed idiotic to idiomatic in previous comment - I hadn't notice autocorrect correcting a typo to something I really did not intend 😬

[mremond]

Thanks @wichert
I quite like the API. The fact that it is simple is nice and you can always have a wrapper in your own project (or in main code) to wait for the result until a timeout.

It feels right to use channel for that need.

I will review the code ASAP.

[wichert]

FWIW I just started using this and it works well for me. I did need to make one last change: add SendIQ to Sender and StreamClient so you can use it from handlers.

[mremond]

Thanks, I am at a conference, so I may take some time before I can review this, but the plan is to sort the code in the pipeline during the next week.