-
Notifications
You must be signed in to change notification settings - Fork 124
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #323 from vector-im/bwindels/service-worker-aborts
Map service worker aborts as network errors, so sync does not halt
- Loading branch information
Showing
7 changed files
with
299 additions
and
145 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| /* | ||
| Copyright 2020 Bruno Windels <bruno@windels.cloud> | ||
| Copyright 2020 The Matrix.org Foundation C.I.C. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| import {HomeServerError, ConnectionError} from "../error.js"; | ||
|
|
||
| export class HomeServerRequest { | ||
| constructor(method, url, sourceRequest, log) { | ||
| this._log = log; | ||
| this._sourceRequest = sourceRequest; | ||
| this._promise = sourceRequest.response().then(response => { | ||
| log?.set("status", response.status); | ||
| // ok? | ||
| if (response.status >= 200 && response.status < 300) { | ||
| log?.finish(); | ||
| return response.body; | ||
| } else { | ||
| if (response.status >= 500) { | ||
| const err = new ConnectionError(`Internal Server Error`); | ||
| log?.catch(err); | ||
| throw err; | ||
| } else if (response.status >= 400 && !response.body?.errcode) { | ||
| const err = new ConnectionError(`HTTP error status ${response.status} without errcode in body, assume this is a load balancer complaining the server is offline.`); | ||
| log?.catch(err); | ||
| throw err; | ||
| } else { | ||
| const err = new HomeServerError(method, url, response.body, response.status); | ||
| log?.set("errcode", err.errcode); | ||
| log?.catch(err); | ||
| throw err; | ||
| } | ||
| } | ||
| }, err => { | ||
| // if this._sourceRequest is still set, | ||
| // the abort error came not from calling abort here | ||
| if (err.name === "AbortError" && this._sourceRequest) { | ||
| // The service worker sometimes (only on Firefox, on long, large request, | ||
| // perhaps it has its own timeout?) aborts the request, see #187. | ||
| // When it happens, the best thing to do seems to be to retry. | ||
| // | ||
| // In the service worker, we will also actively abort all | ||
| // ongoing requests when trying to get a new service worker to activate | ||
| // (this may surface in the app as a TypeError, which already gets mapped | ||
| // to a ConnectionError in the request function, or an AbortError, | ||
| // depending on the browser), as the service worker will only be | ||
| // replaced when there are no more (fetch) events for the | ||
| // current one to handle. | ||
| // | ||
| // In that case, the request function (in fetch.js) will check | ||
| // the haltRequests flag on the service worker handler, and | ||
| // block any new requests, as that would break the update process. | ||
| // | ||
| // So it is OK to return a ConnectionError here. | ||
| // If we're updating the service worker, the /versions polling will | ||
| // be blocked at the fetch level because haltRequests is set. | ||
| // And for #187, retrying is the right thing to do. | ||
| const err = new ConnectionError(`Service worker aborted, either updating or hit #187.`); | ||
| log?.catch(err); | ||
| throw err; | ||
| } else { | ||
| if (err.name === "ConnectionError") { | ||
| log?.set("timeout", err.isTimeout); | ||
| } | ||
| log?.catch(err); | ||
| throw err; | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| abort() { | ||
| if (this._sourceRequest) { | ||
| this._log?.set("aborted", true); | ||
| this._sourceRequest.abort(); | ||
| // to mark that it was on purpose in above rejection handler | ||
| this._sourceRequest = null; | ||
| } | ||
| } | ||
|
|
||
| response() { | ||
| return this._promise; | ||
| } | ||
| } | ||
|
|
||
| import {Request as MockRequest} from "../../mocks/Request.js"; | ||
| import {AbortError} from "../error.js"; | ||
|
|
||
| export function tests() { | ||
| return { | ||
| "Response is passed through": async assert => { | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.respond(200, "foo"); | ||
| assert.equal(await hsRequest.response(), "foo"); | ||
| }, | ||
| "Unexpected AbortError is mapped to ConnectionError": async assert => { | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.reject(new AbortError()); | ||
| await assert.rejects(hsRequest.response(), ConnectionError); | ||
| }, | ||
| "500 response is mapped to ConnectionError": async assert => { | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.respond(500); | ||
| await assert.rejects(hsRequest.response(), ConnectionError); | ||
| }, | ||
| "4xx response is mapped to HomeServerError": async assert => { | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.respond(400, {errcode: "FOO"}); | ||
| await assert.rejects(hsRequest.response(), HomeServerError); | ||
| }, | ||
| "4xx response without errcode is mapped to ConnectionError": async assert => { | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.respond(400); | ||
| await assert.rejects(hsRequest.response(), ConnectionError); | ||
| }, | ||
| "Other errors are passed through": async assert => { | ||
| class MyError extends Error {} | ||
| const request = new MockRequest(); | ||
| const hsRequest = new HomeServerRequest("GET", "https://hs.tld/foo", request); | ||
| request.reject(new MyError()); | ||
| await assert.rejects(hsRequest.response(), MyError); | ||
| }, | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| Copyright 2020 Bruno Windels <bruno@windels.cloud> | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| import {AbortError} from "../utils/error.js"; | ||
|
|
||
| export class Request { | ||
| constructor() { | ||
| this._responsePromise = new Promise((resolve, reject) => { | ||
| this.resolve = resolve; | ||
| this.reject = reject; | ||
| }); | ||
| this.aborted = false; | ||
| } | ||
|
|
||
| respond(status, body) { | ||
| this.resolve({status, body}); | ||
| return this; | ||
| } | ||
|
|
||
| abort() { | ||
| this.aborted = true; | ||
| this.reject(new AbortError()); | ||
| } | ||
|
|
||
| response() { | ||
| return this._responsePromise; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.