From adf361843a05b141c45005c0cd801bf3582b9148 Mon Sep 17 00:00:00 2001 From: Anant Narayanan Date: Wed, 26 Aug 2009 00:05:57 -0700 Subject: [PATCH 1/6] Change Resource.get() semantics and support X-Weave-Alert (bug #478330) --HG-- extra : rebase_source : e0eb1e34f85ddd041005d780590640005dc0b434 --- services/sync/modules/engines.js | 16 +++++++++--- services/sync/modules/resource.js | 21 +++++++++++++-- services/sync/modules/service.js | 43 ++++++++++++++++++++++++++----- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 758e430e413a..803e090b3e47 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -218,7 +218,10 @@ SyncEngine.prototype = { __proto__: Engine.prototype, _recordObj: CryptoWrapper, - + + _alerts: "", + get alerts() { return this._alerts; }, + get baseURL() { let url = Svc.Prefs.get("clusterURL"); if (!url) @@ -341,8 +344,15 @@ SyncEngine.prototype = { Sync.sleep(0); }); - newitems.get(); - + let resp = newitems.get(); + try { + // we only need to store the latest alert + this._alerts = resp.getHeader("X-Weave-Alert"); + } catch (e) { + // no alert headers were present + this._alerts = ""; + } + if (this.lastSync < this._lastSyncTmp) this.lastSync = this._lastSyncTmp; diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js index c4afb9176959..c435832340cb 100644 --- a/services/sync/modules/resource.js +++ b/services/sync/modules/resource.js @@ -50,6 +50,23 @@ Cu.import("resource://weave/constants.js"); Cu.import("resource://weave/util.js"); Cu.import("resource://weave/auth.js"); +// = RequestResponse = +// +// This object is returned on a successful +// HTTP response (codes 200-300) and encapsulates +// returned headers and the actual response text +function RequestResponse(channel, response) { + this._channel = channel; + this._response = response; +} +RequestResponse.prototype = { + get response() { return this._response }, + get status() { return this._channel.responseStatus }, + getHeader: function ReqRe_getHeader(which) { + return this._channel.getResponseHeader(which); + } +}; + // = RequestException = // // This function raises an exception through the call @@ -293,7 +310,7 @@ Resource.prototype = { this._log.debug(action + " request failed (" + channel.responseStatus + ")"); if (this._data) this._log.debug("Error response: " + this._data); - throw new RequestException(this, action, channel); + return new RequestResponse(channel, this._data); } else { this._log.debug(action + " request successful (" + channel.responseStatus + ")"); @@ -313,7 +330,7 @@ Resource.prototype = { } } - return this._data; + return new RequestResponse(channel, this._data); }, // ** {{{ Resource.get }}} ** diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index 094cdefcb707..a91a6c675b69 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -172,6 +172,10 @@ WeaveSvc.prototype = { // Timer object for automagically syncing _syncTimer: null, + // Current alerts + _alerts: "[]", + get alerts() { return JSON.parse(this._alerts); }, + get username() { return Svc.Prefs.get("username", ""); }, @@ -532,17 +536,38 @@ WeaveSvc.prototype = { }; // login may fail because of cluster change + let response; try { - res.get(); + response = res.get(); } catch (e) {} - try { - switch (res.lastChannel.responseStatus) { + if (response) { + switch (response.status) { case 200: - if (passphrase && !this.verifyPassphrase(username, password, passphrase)) { + if (passphrase && + !this.verifyPassphrase(username, password, passphrase)) { this._setSyncFailure(LOGIN_FAILED_INVALID_PASSPHRASE); return false; } + + // check for alerts + try { + let alerts = response.getHeader("X-Weave-Alert"); + if (this._alerts != alerts) { + this._alerts = alerts; + Svc.Observer.notifyObservers( + null, "weave:service:alerts:changed", "" + ); + } + } catch (e) { + // no alert headers were present + if (this.alerts.length != 0) { + this._alerts = ""; + Svc.Observer.notifyObservers( + null, "weave:service:alerts:changed", "" + ); + } + } return true; case 401: if (this.updateCluster(username)) @@ -554,9 +579,9 @@ WeaveSvc.prototype = { default: throw "unexpected HTTP response: " + res.lastChannel.responseStatus; } - } catch (e) { + } else { // if we get here, we have either a busted channel or a network error - this._log.debug("verifyLogin failed: " + e) + this._log.debug("verifyLogin failed: network error"); this._setSyncFailure(LOGIN_FAILED_NETWORK_ERROR); throw e; } @@ -1222,6 +1247,12 @@ WeaveSvc.prototype = { _syncEngine: function WeaveSvc__syncEngine(engine) { try { engine.sync(); + if (this._alerts != engine.alerts) { + this._alerts = engine.alerts; + Svc.Observer.notifyObservers( + null, "weave:service:alerts:changed", "" + ); + } return true; } catch(e) { From 852bb71758ab313d0d9bf6a10f9f283ac63dfefb Mon Sep 17 00:00:00 2001 From: Edward Lee Date: Wed, 26 Aug 2009 01:50:36 -0700 Subject: [PATCH 2/6] Backed out changeset 129ca9a54aed due to burning test_auth_manager: FAIL test_resource: FAIL --- services/sync/modules/engines.js | 16 +++--------- services/sync/modules/resource.js | 21 ++------------- services/sync/modules/service.js | 43 +++++-------------------------- 3 files changed, 11 insertions(+), 69 deletions(-) diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 803e090b3e47..758e430e413a 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -218,10 +218,7 @@ SyncEngine.prototype = { __proto__: Engine.prototype, _recordObj: CryptoWrapper, - - _alerts: "", - get alerts() { return this._alerts; }, - + get baseURL() { let url = Svc.Prefs.get("clusterURL"); if (!url) @@ -344,15 +341,8 @@ SyncEngine.prototype = { Sync.sleep(0); }); - let resp = newitems.get(); - try { - // we only need to store the latest alert - this._alerts = resp.getHeader("X-Weave-Alert"); - } catch (e) { - // no alert headers were present - this._alerts = ""; - } - + newitems.get(); + if (this.lastSync < this._lastSyncTmp) this.lastSync = this._lastSyncTmp; diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js index c435832340cb..c4afb9176959 100644 --- a/services/sync/modules/resource.js +++ b/services/sync/modules/resource.js @@ -50,23 +50,6 @@ Cu.import("resource://weave/constants.js"); Cu.import("resource://weave/util.js"); Cu.import("resource://weave/auth.js"); -// = RequestResponse = -// -// This object is returned on a successful -// HTTP response (codes 200-300) and encapsulates -// returned headers and the actual response text -function RequestResponse(channel, response) { - this._channel = channel; - this._response = response; -} -RequestResponse.prototype = { - get response() { return this._response }, - get status() { return this._channel.responseStatus }, - getHeader: function ReqRe_getHeader(which) { - return this._channel.getResponseHeader(which); - } -}; - // = RequestException = // // This function raises an exception through the call @@ -310,7 +293,7 @@ Resource.prototype = { this._log.debug(action + " request failed (" + channel.responseStatus + ")"); if (this._data) this._log.debug("Error response: " + this._data); - return new RequestResponse(channel, this._data); + throw new RequestException(this, action, channel); } else { this._log.debug(action + " request successful (" + channel.responseStatus + ")"); @@ -330,7 +313,7 @@ Resource.prototype = { } } - return new RequestResponse(channel, this._data); + return this._data; }, // ** {{{ Resource.get }}} ** diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index a91a6c675b69..094cdefcb707 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -172,10 +172,6 @@ WeaveSvc.prototype = { // Timer object for automagically syncing _syncTimer: null, - // Current alerts - _alerts: "[]", - get alerts() { return JSON.parse(this._alerts); }, - get username() { return Svc.Prefs.get("username", ""); }, @@ -536,38 +532,17 @@ WeaveSvc.prototype = { }; // login may fail because of cluster change - let response; try { - response = res.get(); + res.get(); } catch (e) {} - if (response) { - switch (response.status) { + try { + switch (res.lastChannel.responseStatus) { case 200: - if (passphrase && - !this.verifyPassphrase(username, password, passphrase)) { + if (passphrase && !this.verifyPassphrase(username, password, passphrase)) { this._setSyncFailure(LOGIN_FAILED_INVALID_PASSPHRASE); return false; } - - // check for alerts - try { - let alerts = response.getHeader("X-Weave-Alert"); - if (this._alerts != alerts) { - this._alerts = alerts; - Svc.Observer.notifyObservers( - null, "weave:service:alerts:changed", "" - ); - } - } catch (e) { - // no alert headers were present - if (this.alerts.length != 0) { - this._alerts = ""; - Svc.Observer.notifyObservers( - null, "weave:service:alerts:changed", "" - ); - } - } return true; case 401: if (this.updateCluster(username)) @@ -579,9 +554,9 @@ WeaveSvc.prototype = { default: throw "unexpected HTTP response: " + res.lastChannel.responseStatus; } - } else { + } catch (e) { // if we get here, we have either a busted channel or a network error - this._log.debug("verifyLogin failed: network error"); + this._log.debug("verifyLogin failed: " + e) this._setSyncFailure(LOGIN_FAILED_NETWORK_ERROR); throw e; } @@ -1247,12 +1222,6 @@ WeaveSvc.prototype = { _syncEngine: function WeaveSvc__syncEngine(engine) { try { engine.sync(); - if (this._alerts != engine.alerts) { - this._alerts = engine.alerts; - Svc.Observer.notifyObservers( - null, "weave:service:alerts:changed", "" - ); - } return true; } catch(e) { From 6b21954fbd3557d979ad5dc587f34f65cdc3b24e Mon Sep 17 00:00:00 2001 From: Mike Connor Date: Wed, 26 Aug 2009 13:03:33 -0400 Subject: [PATCH 3/6] add missing pick-sync.dtd --HG-- extra : rebase_source : 082544cd2a595206bd550cee94d35f635444e5ed --- services/sync/locales/en-US/fennec.properties | 4 ++-- services/sync/locales/en-US/pick-sync.dtd | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 services/sync/locales/en-US/pick-sync.dtd diff --git a/services/sync/locales/en-US/fennec.properties b/services/sync/locales/en-US/fennec.properties index 50da8e7d19eb..b10ea7e31d3d 100644 --- a/services/sync/locales/en-US/fennec.properties +++ b/services/sync/locales/en-US/fennec.properties @@ -8,11 +8,11 @@ fennec.login.error.detail = Login error: %S fennec.default.client.type = Mobile fennec.sync.start = Syncing Now... -fennec.sync.complete.time = Sync completed at %S, %S +fennec.sync.complete.time = Sync completed at %1$S, %2$S fennec.sync.error.detail = Error: %S fennec.sync.error.generic = Weave had an error when trying to sync. fennec.sync.status = Syncing %S... -fennec.sync.status.detail = Syncing (%S %S)... +fennec.sync.status.detail = Syncing (%1$S %1$S)... fennec.turn.weave.off = Turn Weave Off fennec.turn.weave.on = Turn Weave On diff --git a/services/sync/locales/en-US/pick-sync.dtd b/services/sync/locales/en-US/pick-sync.dtd new file mode 100644 index 000000000000..2977210c2604 --- /dev/null +++ b/services/sync/locales/en-US/pick-sync.dtd @@ -0,0 +1,9 @@ + + + + + + + + + From 2ee5449f0323d2ef072cc946a9d1d71e04964736 Mon Sep 17 00:00:00 2001 From: Mike Connor Date: Wed, 26 Aug 2009 13:07:23 -0400 Subject: [PATCH 4/6] remove files that were supposed to be culled already, stupid Hg --- services/sync/locales/en-US/oauth.dtd | 18 ------------------ services/sync/locales/en-US/oauth.properties | 10 ---------- services/sync/locales/en-US/share.dtd | 6 ------ services/sync/locales/en-US/share.properties | 4 ---- 4 files changed, 38 deletions(-) delete mode 100644 services/sync/locales/en-US/oauth.dtd delete mode 100644 services/sync/locales/en-US/oauth.properties delete mode 100644 services/sync/locales/en-US/share.dtd delete mode 100644 services/sync/locales/en-US/share.properties diff --git a/services/sync/locales/en-US/oauth.dtd b/services/sync/locales/en-US/oauth.dtd deleted file mode 100644 index 87b9843cc37a..000000000000 --- a/services/sync/locales/en-US/oauth.dtd +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/services/sync/locales/en-US/oauth.properties b/services/sync/locales/en-US/oauth.properties deleted file mode 100644 index bb2f02fb718c..000000000000 --- a/services/sync/locales/en-US/oauth.properties +++ /dev/null @@ -1,10 +0,0 @@ -intro.uidmsg = You are logged in as %S -conf.conmsg = A third party identifying itself as %S is requesting access to your Weave Data. -conf.error = Sorry, but the third party's request was invalid: %S -conf.error1 = a request token was not issued. -conf.error2 = the third party is unregistered. -conf.error3 = the request token has expired. -conf.error4 = your account details could not be verified. -final.step1 = Unwrapping symmetric key... -final.step2 = Adding third party to your keyring... -final.step3 = Done! \ No newline at end of file diff --git a/services/sync/locales/en-US/share.dtd b/services/sync/locales/en-US/share.dtd deleted file mode 100644 index c14d4873cf00..000000000000 --- a/services/sync/locales/en-US/share.dtd +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/services/sync/locales/en-US/share.properties b/services/sync/locales/en-US/share.properties deleted file mode 100644 index 9f5914754368..000000000000 --- a/services/sync/locales/en-US/share.properties +++ /dev/null @@ -1,4 +0,0 @@ -status.ok = Weave sharing complete! -status.error = Error. Please check the Weave ID and try again. -status.working = Working... -folder.message = You have chosen to share the bookmark folder "%S". \ No newline at end of file From 1996dc02462ed1471b2bd6ab984d768b5c4f7daf Mon Sep 17 00:00:00 2001 From: Edward Lee Date: Wed, 26 Aug 2009 14:22:11 -0700 Subject: [PATCH 5/6] Bug 506297 - Livemarks with null site/feed uris cause sync to fail It's possible for livemarks to not have a siteURI, so don't assume it to be there. --HG-- extra : rebase_source : 870bb41c980834ef3e5f302739d20adfed6f7f8d --- services/sync/modules/engines/bookmarks.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 2e966de7413b..d93ef982379e 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -365,9 +365,12 @@ BookmarksStore.prototype = { record._insertPos, "as", record.title].join(" ")); break; case "livemark": - newId = this._ls.createLivemark(record._parent, record.title, - Utils.makeURI(record.siteUri), Utils.makeURI(record.feedUri), - record._insertPos); + let siteURI = null; + if (record.siteUri != null) + siteURI = Utils.makeURI(record.siteUri); + + newId = this._ls.createLivemark(record._parent, record.title, siteURI, + Utils.makeURI(record.feedUri), record._insertPos); this._log.debug(["created livemark", newId, "under", record._parent, "at", record._insertPos, "as", record.title, record.siteUri, record.feedUri]. join(" ")); @@ -621,7 +624,10 @@ BookmarksStore.prototype = { case this._bms.TYPE_FOLDER: if (this._ls.isLivemark(placeId)) { record = new Livemark(); - record.siteUri = this._ls.getSiteURI(placeId).spec; + + let siteURI = this._ls.getSiteURI(placeId); + if (siteURI != null) + record.siteUri = siteURI.spec; record.feedUri = this._ls.getFeedURI(placeId).spec; } else { From 771c9805e74bdc608c6b34d6ebccb79d5872137a Mon Sep 17 00:00:00 2001 From: Edward Lee Date: Wed, 26 Aug 2009 15:32:46 -0700 Subject: [PATCH 6/6] Bug 511746 - Resource.foo shouldn't throw except in exceptional cases. r=thunder Get rid of lastChannel and return a String object from _request with additional properties of status, succeeded, headers -- even if the response was handled by cache. Update engines to check for non-success and throw the failure. Update tests to use these additional properties instead of lastChannel, etc. --- services/sync/modules/base_records/wbo.js | 9 +- services/sync/modules/engines.js | 13 ++- services/sync/modules/resource.js | 103 +++++++----------- services/sync/modules/service.js | 97 ++++++++--------- services/sync/tests/unit/test_auth_manager.js | 2 +- services/sync/tests/unit/test_records_keys.js | 4 +- services/sync/tests/unit/test_records_wbo.js | 6 +- services/sync/tests/unit/test_resource.js | 33 +++--- 8 files changed, 125 insertions(+), 142 deletions(-) diff --git a/services/sync/modules/base_records/wbo.js b/services/sync/modules/base_records/wbo.js index 002786a1f127..50989447ae6d 100644 --- a/services/sync/modules/base_records/wbo.js +++ b/services/sync/modules/base_records/wbo.js @@ -131,12 +131,13 @@ RecordManager.prototype = { import: function RecordMgr_import(url) { this._log.trace("Importing record: " + (url.spec ? url.spec : url)); try { - this.lastResource = new Resource(url); - this.lastResource.get(); + // Clear out the last response with empty object if GET fails + this.response = {}; + this.response = new Resource(url).get(); let record = new this._recordType(); - record.deserialize(this.lastResource.data); - record.uri = url; // NOTE: may override id in this.lastResource.data + record.deserialize(this.response); + record.uri = url; return this.set(url, record); } diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 758e430e413a..709f12aa2090 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -283,7 +283,9 @@ SyncEngine.prototype = { meta.generateIV(); meta.addUnwrappedKey(pubkey, symkey); let res = new Resource(meta.uri); - res.put(meta.serialize()); + let resp = res.put(meta.serialize()); + if (!resp.success) + throw resp; // Cache the cryto meta that we just put on the server CryptoMetas.set(meta.uri, meta); @@ -341,7 +343,9 @@ SyncEngine.prototype = { Sync.sleep(0); }); - newitems.get(); + let resp = newitems.get(); + if (!resp.success) + throw resp; if (this.lastSync < this._lastSyncTmp) this.lastSync = this._lastSyncTmp; @@ -462,7 +466,10 @@ SyncEngine.prototype = { // Upload what we've got so far in the collection let doUpload = Utils.bind2(this, function(desc) { this._log.info("Uploading " + desc + " of " + outnum + " records"); - up.post(); + let resp = up.post(); + if (!resp.success) + throw resp; + if (up.data.modified > this.lastSync) this.lastSync = up.data.modified; up.clearRecords(); diff --git a/services/sync/modules/resource.js b/services/sync/modules/resource.js index c4afb9176959..6e522371d9ce 100644 --- a/services/sync/modules/resource.js +++ b/services/sync/modules/resource.js @@ -50,27 +50,6 @@ Cu.import("resource://weave/constants.js"); Cu.import("resource://weave/util.js"); Cu.import("resource://weave/auth.js"); -// = RequestException = -// -// This function raises an exception through the call -// stack for a failed network request. -function RequestException(resource, action, request) { - this._resource = resource; - this._action = action; - this._request = request; - this.location = Components.stack.caller; -} -RequestException.prototype = { - get resource() { return this._resource; }, - get action() { return this._action; }, - get request() { return this._request; }, - get status() { return this._request.status; }, - toString: function ReqEx_toString() { - return "Could not " + this._action + " resource " + this._resource.spec + - " (" + this._request.responseStatus + ")"; - } -}; - // = Resource = // // Represents a remote network resource, identified by a URI. @@ -122,8 +101,6 @@ Resource.prototype = { return this._uri; }, set uri(value) { - this._dirty = true; - this._downloaded = false; if (typeof value == 'string') this._uri = Utils.makeURI(value); else @@ -145,17 +122,9 @@ Resource.prototype = { _data: null, get data() this._data, set data(value) { - this._dirty = true; this._data = value; }, - _lastChannel: null, - _downloaded: false, - _dirty: false, - get lastChannel() this._lastChannel, - get downloaded() this._downloaded, - get dirty() this._dirty, - // ** {{{ Resource.filters }}} ** // // Filters are used to perform pre and post processing on @@ -188,18 +157,15 @@ Resource.prototype = { // to obtain a request channel. // _createRequest: function Res__createRequest() { - this._lastChannel = Svc.IO.newChannel(this.spec, null, null). - QueryInterface(Ci.nsIRequest); + let channel = Svc.IO.newChannel(this.spec, null, null). + QueryInterface(Ci.nsIRequest).QueryInterface(Ci.nsIHttpChannel); // Always validate the cache: - let loadFlags = this._lastChannel.loadFlags; - loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE; - loadFlags |= Ci.nsIRequest.INHIBIT_CACHING; - this._lastChannel.loadFlags = loadFlags; - this._lastChannel = this._lastChannel.QueryInterface(Ci.nsIHttpChannel); + channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE; + channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING; // Setup a callback to handle bad HTTPS certificates. - this._lastChannel.notificationCallbacks = new badCertListener(); + channel.notificationCallbacks = new badCertListener(); // Avoid calling the authorizer more than once let headers = this.headers; @@ -208,9 +174,9 @@ Resource.prototype = { this._log.trace("HTTP Header " + key + ": ***** (suppressed)"); else this._log.trace("HTTP Header " + key + ": " + headers[key]); - this._lastChannel.setRequestHeader(key, headers[key], false); + channel.setRequestHeader(key, headers[key], false); } - return this._lastChannel; + return channel; }, _onProgress: function Res__onProgress(channel) {}, @@ -289,31 +255,44 @@ Resource.prototype = { throw error; } - if (!channel.requestSucceeded) { - this._log.debug(action + " request failed (" + channel.responseStatus + ")"); - if (this._data) - this._log.debug("Error response: " + this._data); - throw new RequestException(this, action, channel); - - } else { - this._log.debug(action + " request successful (" + channel.responseStatus + ")"); - - switch (action) { - case "DELETE": - if (Utils.checkStatus(channel.responseStatus, null, [[200,300],404])){ - this._dirty = false; - this._data = null; + // Set some default values in-case there's no response header + let headers = {}; + let status = 0; + let success = true; + try { + // Read out the response headers if available + channel.visitResponseHeaders({ + visitHeader: function visitHeader(header, value) { + headers[header] = value; + } + }); + status = channel.responseStatus; + success = channel.requestSucceeded; + + if (success) { + this._log.debug(action + " success: " + status); + switch (action) { + case "GET": + case "POST": + if (this._log.level <= Log4Moz.Level.Trace) + this._log.trace(action + " Body: " + this._data); + this.filterDownload(); + break; } - break; - case "GET": - case "POST": - this._log.trace(action + " Body: " + this._data); - this.filterDownload(); - break; } + else + this._log.debug(action + " fail: " + status + " " + this._data); + } + // Got a response but no header; must be cached (use default values) + catch(ex) { + this._log.debug(action + " cached: " + status); } - return this._data; + let ret = new String(this._data); + ret.headers = headers; + ret.status = status; + ret.success = success; + return ret; }, // ** {{{ Resource.get }}} ** diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index 094cdefcb707..eb31147522f0 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -460,18 +460,16 @@ WeaveSvc.prototype = { let res = new Resource(this.baseURL + "api/register/chknode/" + username); try { - res.get(); - } catch(ex) {} - - try { - switch (res.lastChannel.responseStatus) { + let node = res.get(); + switch (node.status) { case 404: this._log.debug("Using serverURL as data cluster (multi-cluster support disabled)"); return Svc.Prefs.get("serverURL"); + case 0: case 200: - return "https://" + res.data + "/"; + return "https://" + node + "/"; default: - this._log.debug("Unexpected response code trying to find cluster: " + res.lastChannel.responseStatus); + this._log.debug("Unexpected response code: " + node.status); break; } } catch (e) { @@ -533,11 +531,8 @@ WeaveSvc.prototype = { // login may fail because of cluster change try { - res.get(); - } catch (e) {} - - try { - switch (res.lastChannel.responseStatus) { + let test = res.get(); + switch (test.status) { case 200: if (passphrase && !this.verifyPassphrase(username, password, passphrase)) { this._setSyncFailure(LOGIN_FAILED_INVALID_PASSPHRASE); @@ -552,7 +547,7 @@ WeaveSvc.prototype = { this._log.debug("verifyLogin failed: login failed") return false; default: - throw "unexpected HTTP response: " + res.lastChannel.responseStatus; + throw "unexpected HTTP response: " + test.status; } } catch (e) { // if we get here, we have either a busted channel or a network error @@ -595,7 +590,10 @@ WeaveSvc.prototype = { privkey.payload.iv, newphrase); privkey.payload.keyData = newkey; - new Resource(privkey.uri).put(privkey.serialize()); + let resp = new Resource(privkey.uri).put(privkey.serialize()); + if (!resp.success) + throw resp; + this.passphrase = newphrase; return true; @@ -613,7 +611,7 @@ WeaveSvc.prototype = { "Content-Length", message.length); let resp = res.post(message); - if (res.lastChannel.responseStatus != 200) { + if (resp.status != 200) { this._log.info("Password change failed: " + resp); throw "Could not change password"; } @@ -740,7 +738,7 @@ WeaveSvc.prototype = { }, _errorStr: function WeaveSvc__errorStr(code) { - switch (code) { + switch (code.toString()) { case "0": return "uid-in-use"; case "-1": @@ -782,7 +780,7 @@ WeaveSvc.prototype = { let data = ""; try { data = res.get(); - if (res.lastChannel.responseStatus == 200 && data == "0") + if (data.status == 200 && data == "0") return "available"; } catch(ex) {} @@ -804,31 +802,29 @@ WeaveSvc.prototype = { res.setHeader("Content-Type", "application/x-www-form-urlencoded", "Content-Length", message.length); - let ret = {}; + let error = "generic-server-error"; try { - ret.response = res.post(message); - ret.status = res.lastChannel.responseStatus; + let register = res.post(message); + if (register.success) { + this._log.info("Account created: " + register); + return; + } - // No exceptions must have meant it was successful - this._log.info("Account created: " + ret.response); - return ret; + // Must have failed, so figure out the reason + switch (register.status) { + case 400: + error = this._errorStr(register); + break; + case 417: + error = "captcha-incorrect"; + break; + } } catch(ex) { this._log.warn("Failed to create account: " + ex); - let status = ex.request.responseStatus; - switch (status) { - case 400: - ret.error = this._errorStr(status); - break; - case 417: - ret.error = "captcha-incorrect"; - break; - default: - ret.error = "generic-server-error"; - break; - } - return ret; } + + return error; }, // stuff we need to to after login, before we can really do @@ -849,7 +845,7 @@ WeaveSvc.prototype = { Svc.Version.compare(COMPATIBLE_VERSION, remoteVersion) > 0) { // abort the server wipe if the GET status was anything other than 404 or 200 - let status = Records.lastResource.lastChannel.responseStatus; + let status = Records.response.status; if (status != 200 && status != 404) { this._setSyncFailure(METARECORD_DOWNLOAD_FAIL); this._log.warn("Unknown error while downloading metadata record. " + @@ -918,14 +914,10 @@ WeaveSvc.prototype = { } if (needKeys) { - if (PubKeys.lastResource != null && PrivKeys.lastResource != null && - PubKeys.lastResource.lastChannel.responseStatus != 404 && - PrivKeys.lastResource.lastChannel.responseStatus != 404) { + if (PubKeys.response.status != 404 && PrivKeys.response.status != 404) { this._log.warn("Couldn't download keys from server, aborting sync"); - this._log.debug("PubKey HTTP response status: " + - PubKeys.lastResource.lastChannel.responseStatus); - this._log.debug("PrivKey HTTP response status: " + - PrivKeys.lastResource.lastChannel.responseStatus); + this._log.debug("PubKey HTTP status: " + PubKeys.response.status); + this._log.debug("PrivKey HTTP status: " + PrivKeys.response.status); this._setSyncFailure(KEYS_DOWNLOAD_FAIL); return false; } @@ -956,8 +948,6 @@ WeaveSvc.prototype = { } catch (e) { this._setSyncFailure(KEYS_UPLOAD_FAIL); this._log.error("Could not upload keys: " + Utils.exceptionStr(e)); - // FIXME no lastRequest anymore - //this._log.error(keys.pubkey.lastRequest.responseText); } } else { this._setSyncFailure(SETUP_FAILED_NO_PASSPHRASE); @@ -1073,7 +1063,8 @@ WeaveSvc.prototype = { // this is sort of pseudocode, need a way to get at the if (!shouldBackoff) { try { - shouldBackoff = Utils.checkStatus(Records.lastResource.lastChannel.responseStatus, null, [500,[502,504]]); + shouldBackoff = Utils.checkStatus(Records.response.status, null, + [500, [502, 504]]); } catch (e) { // if responseStatus throws, we have a network issue in play @@ -1226,10 +1217,9 @@ WeaveSvc.prototype = { } catch(e) { // maybe a 401, cluster update needed? - if (e.constructor.name == "RequestException" && e.status == 401) { - if (this.updateCluster(this.username)) - return this._syncEngine(engine); - } + if (e.status == 401 && this.updateCluster(this.username)) + return this._syncEngine(engine); + this._syncError = true; this._weaveStatusCode = WEAVE_STATUS_PARTIAL; this._detailedStatus.setEngineStatus(engine.name, e); @@ -1260,8 +1250,9 @@ WeaveSvc.prototype = { this._log.debug("Setting meta payload storage version to " + WEAVE_VERSION); meta.payload.storageVersion = WEAVE_VERSION; - let res = new Resource(meta.uri); - res.put(meta.serialize()); + let resp = new Resource(meta.uri).put(meta.serialize()); + if (!resp.success) + throw resp; }, /** diff --git a/services/sync/tests/unit/test_auth_manager.js b/services/sync/tests/unit/test_auth_manager.js index 310d1eceb7ad..170cc6260523 100644 --- a/services/sync/tests/unit/test_auth_manager.js +++ b/services/sync/tests/unit/test_auth_manager.js @@ -41,7 +41,7 @@ function run_test() { let res = new Resource("http://localhost:8080/foo"); let content = res.get(); do_check_eq(content, "This path exists and is protected"); - do_check_eq(res.lastChannel.responseStatus, 200); + do_check_eq(content.status, 200); server.stop(); } diff --git a/services/sync/tests/unit/test_records_keys.js b/services/sync/tests/unit/test_records_keys.js index b336127267e0..90b56ca35c9f 100644 --- a/services/sync/tests/unit/test_records_keys.js +++ b/services/sync/tests/unit/test_records_keys.js @@ -43,13 +43,13 @@ function run_test() { let pubkey = PubKeys.get("http://localhost:8080/pubkey"); do_check_eq(pubkey.data.payload.type, "pubkey"); - do_check_eq(PubKeys.lastResource.lastChannel.responseStatus, 200); + do_check_eq(PubKeys.response.status, 200); log.info("Getting matching private key"); let privkey = PrivKeys.get(pubkey.privateKeyUri); do_check_eq(privkey.data.payload.type, "privkey"); - do_check_eq(PrivKeys.lastResource.lastChannel.responseStatus, 200); + do_check_eq(PrivKeys.response.status, 200); log.info("Done!"); } diff --git a/services/sync/tests/unit/test_records_wbo.js b/services/sync/tests/unit/test_records_wbo.js index 65d7b846a397..994a60ffe4c9 100644 --- a/services/sync/tests/unit/test_records_wbo.js +++ b/services/sync/tests/unit/test_records_wbo.js @@ -48,7 +48,7 @@ function run_test() { log.info("Getting a WBO record"); let res = new Resource("http://localhost:8080/record"); - res.get(); + let resp = res.get(); let rec = new WBORecord(); rec.deserialize(res.data); @@ -60,7 +60,7 @@ function run_test() { do_check_eq(rec.modified, 2454725.98283); do_check_eq(typeof(rec.payload), "object"); do_check_eq(rec.payload.cheese, "roquefort"); - do_check_eq(res.lastChannel.responseStatus, 200); + do_check_eq(resp.status, 200); log.info("Getting a WBO record using the record manager"); @@ -69,7 +69,7 @@ function run_test() { do_check_eq(rec2.modified, 2454725.98284); do_check_eq(typeof(rec2.payload), "object"); do_check_eq(rec2.payload.cheese, "gruyere"); - do_check_eq(Records.lastResource.lastChannel.responseStatus, 200); + do_check_eq(Records.response.status, 200); log.info("Done!"); } diff --git a/services/sync/tests/unit/test_resource.js b/services/sync/tests/unit/test_resource.js index b035d7b16fc2..8218070e69a6 100644 --- a/services/sync/tests/unit/test_resource.js +++ b/services/sync/tests/unit/test_resource.js @@ -56,14 +56,15 @@ function run_test() { let res = new Resource("http://localhost:8080/open"); let content = res.get(); do_check_eq(content, "This path exists"); - do_check_eq(res.lastChannel.responseStatus, 200); + do_check_eq(content.status, 200); + do_check_true(content.success); - // 2. A password protected resource (test that it'll fail w/o pass) + // 2. A password protected resource (test that it'll fail w/o pass, no throw) let res2 = new Resource("http://localhost:8080/protected"); - try { - content = res2.get(); - do_check_true(false); // unreachable, get() above must fail - } catch (e) {} + content = res2.get(); + do_check_eq(content, "This path exists and is protected - failed") + do_check_eq(content.status, 401); + do_check_false(content.success); // 3. A password protected resource @@ -72,17 +73,21 @@ function run_test() { res3.authenticator = auth; content = res3.get(); do_check_eq(content, "This path exists and is protected"); - do_check_eq(res3.lastChannel.responseStatus, 200); + do_check_eq(content.status, 200); + do_check_true(content.success); - // 4. A non-existent resource (test that it'll fail) + // 4. A non-existent resource (test that it'll fail, but not throw) let res4 = new Resource("http://localhost:8080/404"); - try { - let content = res4.get(); - do_check_true(false); // unreachable, get() above must fail - } catch (e) {} - do_check_eq(res4.lastChannel.responseStatusText, "Not Found"); - do_check_eq(res4.lastChannel.responseStatus, 404); + content = res4.get(); + do_check_eq(content, "File not found"); + do_check_eq(content.status, 404); + do_check_false(content.success); + + _("5. Check some headers of the 404 response"); + do_check_eq(content.headers.Connection, "close"); + do_check_eq(content.headers.Server, "httpd.js"); + do_check_eq(content.headers["Content-Length"], 14); // FIXME: additional coverage needed: // * PUT requests