Opened 8 years ago

Closed 8 years ago

#2218 enhancement closed fixed (fixed)

Add timeout to XMPP IQ requests

Reported by: ralphm Owned by: ralphm
Priority: highest Milestone: Words-0.5
Component: words Keywords:
Cc: ralphm Branch:
Author: Launchpad Bug:

Description

XMPP <iq/> stanzas are designed to have request-response behaviour. The current implementation of t.w.p.j.xmlstream.IQ has a send() method that returns a deferred which fires when a response has been received.

However, if for some reason the other party is too slow or unable to respond, it is desirable to have those deferreds fire with a failure that indicates no response has been received within a configured time period.

Note that the current implementation already fires the deferreds of all pending requests when the XML stream connection has been lost.

Change History (6)

comment:1 Changed 8 years ago by ralphm

  • Keywords review added
  • Owner ralphm deleted
  • Priority changed from normal to highest

Implemented in source:branches/xmpp-iq-timeout-2218.

To test timeouts, I used t.i.task.Clock, and uncovered that it doesn't set its called attribute properly, as mentioned in #2222. This branch contains a fix for this.

Please review.

comment:2 Changed 8 years ago by exarkun

  • Owner set to exarkun
  • Status changed from new to assigned

comment:3 Changed 8 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to ralphm
  • Status changed from assigned to new

Fix to task.py looks good. Timeout support added to jabber mostly looks good. I wonder what will happen if a response is received after the timeout expires, though. Add a test for this case?

comment:4 Changed 8 years ago by ralphm

  • Keywords review added
  • Owner changed from ralphm to exarkun

Nothing happens, since the iq's id is removed from the iqDeferreds dictionary. The existing tests already test this. I did add a test for making sure untracked iq responses (id not in that dictionary) are passed untouched to the next observer.

comment:5 Changed 8 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to ralphm

Looks good. Go ahead and merge.

comment:6 Changed 8 years ago by ralphm

  • Resolution set to fixed
  • Status changed from new to closed

(In [18644]) Add timeout behaviour to xmpp iq requests and make t.i.Clock set call.called.

Author: ralphm
Reviewer: exarkun
Fixes #2218 and #2222

Note: See TracTickets for help on using tickets.