Closed Bug 1415222 Opened 7 years ago Closed 4 years ago

storage.sync engine should back off if we get a request timeout or 500 responses

Categories

(WebExtensions :: Storage, enhancement, P3)

Product:
WebExtensions
Component:
Storage
Version:
49 Branch
Type:
enhancement

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1634615

People

(Reporter: glasserc, Unassigned)

References

Details

If Kinto doesn't respond to a request within our timeout, that could be a sign that we're under heavy load. If that happens, emit a backoff event so that we don't hammer the server.

We might need to bump the request timeout from 30s to be sure we don't interfere with syncing on slow connections.
Via another bug, I'm seeing logs where most requests failed with a timeout, but occasionally with:

> 2017-11-07 06:07:29.939000 (15:24:17.357)	Sync.Engine.Extension-Storage	ERROR	Syncing firefox@ghostery.com: request failed: Error: HTTP 500; SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data (resource://services-common/kinto-http-client.js:2354:21) JS Stack trace: processResponse@kinto-http-client.js:2352:44 < async*request@kinto-http-client.js:2432:14 < async*execute@kinto-http-client.js:745:26 < async*paginatedList@kinto-http-client.js:851:34 < async*listRecords@kinto-http-client.js:2066:14 < async*[6]</pullChanges/<@kinto-offline-client.js:1764:45 < step@kinto-offline-client.js:875:183 < [6]</_asyncToGenerator/</</step/<@kinto-offline-client.js:875:361 < promise callback*step@kinto-offline-client.js:875:314 < [6]</_asyncToGenerator/</<@kinto-offline-client.js:875:437 < [6]</_asyncToGenerator/<@kinto-offline-client.js:875:99 < pullChanges@kinto-offline-client.js:1738:12 < [6]</sync/<@kinto-offline-client.js:2022:15 < step@kinto-offline-client.js:875:183 < [6]</_asyncToGenerator/</<@kinto-offline-client.js:875:437 < [6]</_asyncToGenerator/<@kinto-offline-client.js:875:99 < sync@kinto-offline-client.js:2001:12 < _syncCollection/<@ExtensionStorageSync.jsm:821:14 < _requestWithToken@ExtensionStorageSync.jsm:832:20 < async*_syncCollection@ExtensionStorageSync.jsm:813:12 < sync@ExtensionStorageSync.jsm:751:27 < async*syncAll/promises</<@ExtensionStorageSync.jsm:725:16 < promise callback*syncAll/promises<@ExtensionStorageSync.jsm:724:14 < syncAll@ExtensionStorageSync.jsm:723:22 < async*_sync@extension-storage.js:40:12 < async*WrappedNotify@util.js:169:27 < async*sync@engines.js:726:12 < async*_syncEngine@enginesync.js:203:13 < async*sync@enginesync.js:153:21 < async*onNotify@service.js:1101:13 < async*WrappedNotify@util.js:169:27
> 2017-11-07 06:07:29.941000 (15:24:17.359)	Sync.Engine.Extension-Storage	WARN	Syncing failed: Error: HTTP 500; SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data (resource://services-common/kinto-http-client.js:2354:21) JS Stack trace: processResponse@kinto-http-client.js:2352:44 < async*request@kinto-http-client.js:2432:14 < async*execute@kinto-http-client.js:745:26 < async*paginatedList@kinto-http-client.js:851:34 < async*listRecords@kinto-http-client.js:2066:14 < async*[6]</pullChanges/<@kinto-offline-client.js:1764:45 < step@kinto-offline-client.js:875:183 < [6]</_asyncToGenerator/</</step/<@kinto-offline-client.js:875:361 < promise callback*step@kinto-offline-client.js:875:314 < [6]</_asyncToGenerator/</<@kinto-offline-client.js:875:437 < [6]</_asyncToGenerator/<@kinto-offline-client.js:875:99 < pullChanges@kinto-offline-client.js:1738:12 < [6]</sync/<@kinto-offline-client.js:2022:15 < step@kinto-offline-client.js:875:183 < [6]</_asyncToGenerator/</<@kinto-offline-client.js:875:437 < [6]</_asyncToGenerator/<@kinto-offline-client.js:875:99 < sync@kinto-offline-client.js:2001:12 < _syncCollection/<@ExtensionStorageSync.jsm:821:14 < _requestWithToken@ExtensionStorageSync.jsm:832:20 < async*_syncCollection@ExtensionStorageSync.jsm:813:12 < sync@ExtensionStorageSync.jsm:751:27 < async*syncAll/promises</<@ExtensionStorageSync.jsm:725:16 < promise callback*syncAll/promises<@ExtensionStorageSync.jsm:724:14 < syncAll@ExtensionStorageSync.jsm:723:22 < async*_sync@extension-storage.js:40:12 < async*WrappedNotify@util.js:169:27 < async*sync@engines.js:726:12 < async*_syncEngine@enginesync.js:203:13 < async*sync@enginesync.js:153:21 < async*onNotify@service.js:1101:13 < async*WrappedNotify@util.js:169:27

So I suspect we should consider doing the same for 500 errors.
Summary: storage.sync engine should back off if we get a request timeout → storage.sync engine should back off if we get a request timeout or 500 responses
Component: Sync → WebExtensions: Storage
Product: Firefox → Toolkit
Priority: -- → P3
Product: Toolkit → WebExtensions

Mark, is this bug still relevant for the new storage.sync implementation?
if it has been fixed as part of the rewrite, can we close it as a duplicate of the bug that enabled the new storage.sync implementation?

Flags: needinfo?(markh)
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(markh)
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.