From bug 748517 comment 14: I feel we should reverse the ping flow. Send current ping first, followed by old pings. We can then abort on the first failed ping instead of hammering an upset/missing server as we do now. Should also record success/fail with every ping. This could almost be done concurrently with resending telemetry pings (bug 748520), but can certainly be implemented separately.

Make it part of the iteration logic, rather than hidden away in some handler somewhere.

We should need to query the channel; the kind of event handler called tells us all we need to know about success/fail. Not strictly part of this patch, but a nice cleanup.

Comment on attachment 632668 [details] [diff] [review] part 1 - abort sending after first failed ping + function onSuccess() { + this.sendPingsFromIterator(server, i); + } at first I thought this was horrible, but on second thought chaining iterators via callbacks is kinda cool. + onSuccess.bind(this), onError.bind(this)); I wish JS had a mode to do this by default :(

(In reply to Taras Glek (:taras) from comment #6) > at first I thought this was horrible, but on second thought chaining > iterators via callbacks is kinda cool. The reason I thought it was terrible is that there is no comment explaining the need for complexity, please add a comment describing the purpose of the function.

(In reply to Taras Glek (:taras) from comment #7) > (In reply to Taras Glek (:taras) from comment #6) > > at first I thought this was horrible, but on second thought chaining > > iterators via callbacks is kinda cool. > > The reason I thought it was terrible is that there is no comment explaining > the need for complexity, please add a comment describing the purpose of the > function. I suppose it's not strictly necessary, and the callbacks, at least, could be moved into the event handlers for the ping request, if not the whole iteration logic. I think it is slightly simpler this way, though a bit confusing at first; I had to write out a little diagram before I arrived at what I had now. Will add a comment.

Comment on attachment 632669 [details] [diff] [review] part 2 - record success/fail for every telemetry ping I wonder if these would be more appropriate in the onSuccess/onError handlers in patch 1.

https://hg.mozilla.org/integration/mozilla-inbound/rev/a43806df7029 https://hg.mozilla.org/integration/mozilla-inbound/rev/fa4aed87db0a https://hg.mozilla.org/integration/mozilla-inbound/rev/7227e77dd8d4 https://hg.mozilla.org/integration/mozilla-inbound/rev/c570feae74ce https://hg.mozilla.org/integration/mozilla-inbound/rev/15d0bf6d3d8e

https://hg.mozilla.org/mozilla-central/rev/a43806df7029 https://hg.mozilla.org/mozilla-central/rev/fa4aed87db0a https://hg.mozilla.org/mozilla-central/rev/7227e77dd8d4 https://hg.mozilla.org/mozilla-central/rev/c570feae74ce https://hg.mozilla.org/mozilla-central/rev/15d0bf6d3d8e