daleharvey/pouchdb — pull request #1559 by ig3 (2014-03-03 14:26:43+00:00)
Title: (#1507) Fix changes in adapter.js

This fixes AbstractPouchDB.prototype.changes in adapter.js to pass test fire-complete-on-cancel.
In passing, changes in adapter.js (but not http.js) returns a full promise, detects more errors and cancels changes when promise.cancel is called in more cases (including but not limited to the case tested in fire-complete-on-cancel), to the extent that the various adapters implement their cancel functions correctly. Several cases of recursive calls to db.changes were eliminated, to eliminate errors in their implementations (promise cancel and resolution not propagated effectively) and to avoid nested promises generally.

The promise implementation uses no features of Promise not already in use in utils.toPromise: new Promise and the then function.

For consistency with prior implementation, on cancel the promise is fulfilled with {status: 'cancelled'}, though I note that draft Cancellation/A+ (https://github.com/promises-aplus/cancellation-spec/issues/7) indicates the promise should be rejected with an error on cancel. If a decision is made to align PouchDB's implementation of cancel with Cancellation/A+, the change required to AbstractPouchDB.prototype.changes will be trivial.

AbstractPouchDB.prototype.changes uses a toPromise function, but it is a modification of that in utils. It passes the promise to the called function, along with opts and callback, so that the called function can modify the cancel function as necessary to implement an immediate active cancellation, rather than merely setting a flag which may not be acted on for an indefinite time after it is set (not ever, in some cases I have seen testing replicate). This, in fact, was one of my primary motivations for making this change: to eliminate a case where cancelling replicate hangs indefinitely, waiting to cancellation of changes which never happens.