Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#3310 closed defect (fixed)

Incorrect backlog handling in cometd.js

Reported by: gregwilkins Owned by: alex
Priority: high Milestone: 0.9beta
Component: Dojox Version: 0.9
Keywords: cometd Cc:
Blocked By: Blocking:

Description

The publish method in cometd add messages to the backlog if the state is not connected. However, with the implementation of advice, the client may not be connected for an interval. The backlog is only sent to the server when a connect completes, not on a reconnect, so the messages are effectly lost if they are sent between reconnects.

One solution would be to send the backlog on reconnect. However there is no reason that outgoing messages need to be delayed for an connected status. So the attached patch takes the approach of adding an initialized state, and once the client is initialized (after the first connect) the backlog is sent and messages are no longer backlogged (at least until batching is implemented).

The patch also removed the duplication of connected status in both cometd and transport objects.

It also fixes a few dojox.io.cometd names that had not been updated.

finally the patch also implements disconnect method and sends this on page unload.

(NB. if you want all these fixes as separate patches... I can do that also)

Attachments (2)

dojox.cometd.patch (8.9 KB) - added by gregwilkins 12 years ago.
patch as described in text
dojox.cometd-3310-2.patch (8.4 KB) - added by gregwilkins 12 years ago.
The patch updated for the new arg ordering

Download all attachments as: .zip

Change History (9)

Changed 12 years ago by gregwilkins

Attachment: dojox.cometd.patch added

patch as described in text

comment:1 Changed 12 years ago by gregwilkins

The patch also deletes references to auth fields which are no longer in the spec.

comment:2 Changed 12 years ago by guest

The disconnect has no immediate effect and fire a Syntax error because of empty dfd.ioArgs.xhr.responseText. Should I open an ticket for this ?

-- ybart

comment:3 Changed 12 years ago by guest

According to the current Bayeux spec, this should rather be fixed on the server (however, this should not have produced a syntax error)

comment:4 Changed 12 years ago by James Burke

Owner: changed from alex@… to alex

Changed 12 years ago by gregwilkins

Attachment: dojox.cometd-3310-2.patch added

The patch updated for the new arg ordering

comment:5 Changed 12 years ago by gregwilkins

I have freshened up this patch to handle the latest update with init and subscribe arguments

comment:6 Changed 12 years ago by alex

Resolution: fixed
Status: newclosed

(In [9211]) another awesome patch from Greg Wilkins (CLA on file). The man is on fire. Fixes #3310

comment:7 Changed 12 years ago by alex

Milestone: 0.9beta
Note: See TracTickets for help on using tickets.