From c1f77211974dd3a03b218160755fe2b3d65a077c Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 5 Dec 2019 17:17:57 -0500 Subject: [PATCH] Reload API status automatically when a response indicates a probable change in status (close #6650) Make API status messages more informative (close #7021) Add manual Retry button to "unable to connect" API status message (close #5864) --- css/80_app.css | 12 +++--- data/core.yaml | 12 +++--- dist/locales/en.json | 13 +++--- modules/services/osm.js | 34 ++++++++++++++- modules/ui/status.js | 91 +++++++++++++++++++++++++---------------- 5 files changed, 110 insertions(+), 52 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 507f82740..44588d136 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -4607,7 +4607,7 @@ img.tile-debug { .api-status { text-align: right; - padding: 0px 10px; + padding: 1px 10px; color: #eee; flex: 1 1 auto; } @@ -4621,11 +4621,13 @@ img.tile-debug { background: #a22; } -.api-status-login { - color: #aaf; +.api-status a { + text-decoration: underline; + color: #ccc; + pointer-events: all; } -.api-status-login:hover { - color: #ccf; +.api-status a:hover { + color: inherit; } /* Notification Badges diff --git a/data/core.yaml b/data/core.yaml index b21e24183..831ab099d 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -415,11 +415,13 @@ en: feature_info: hidden_warning: "{count} hidden features" hidden_details: "These features are currently hidden: {details}" - status: - error: Unable to connect to API. - offline: The API is offline. Please try editing later. - readonly: The API is read-only. You will need to wait to save your changes. - rateLimit: The API is limiting anonymous connections. You can fix this by logging in. + osm_api_status: + message: + error: Unable to reach the OpenStreetMap API. Your edits are safe locally. Check your network connection. + offline: The OpenStreetMap API is offline. Your edits are safe locally. Please come back later. + readonly: The OpenStreetMap API is currently read-only. You can continue editing, but must wait to save your changes. + rateLimit: The OpenStreetMap API is limiting anonymous connections. You can fix this by logging in. + retry: Retry commit: title: Upload to OpenStreetMap upload_explanation: "The changes you upload will be visible on all maps that use OpenStreetMap data." diff --git a/dist/locales/en.json b/dist/locales/en.json index 23ee0fdcf..e00c530ee 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -534,11 +534,14 @@ "hidden_warning": "{count} hidden features", "hidden_details": "These features are currently hidden: {details}" }, - "status": { - "error": "Unable to connect to API.", - "offline": "The API is offline. Please try editing later.", - "readonly": "The API is read-only. You will need to wait to save your changes.", - "rateLimit": "The API is limiting anonymous connections. You can fix this by logging in." + "osm_api_status": { + "message": { + "error": "Unable to reach the OpenStreetMap API. Your edits are safe locally. Check your network connection.", + "offline": "The OpenStreetMap API is offline. Your edits are safe locally. Please come back later.", + "readonly": "The OpenStreetMap API is currently read-only. You can continue editing, but must wait to save your changes.", + "rateLimit": "The OpenStreetMap API is limiting anonymous connections. You can fix this by logging in." + }, + "retry": "Retry" }, "commit": { "title": "Upload to OpenStreetMap", diff --git a/modules/services/osm.js b/modules/services/osm.js index 484a4bce4..f48adc5ba 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -13,7 +13,7 @@ import { utilArrayChunk, utilArrayGroupBy, utilArrayUniq, utilRebind, utilTiler, var tiler = utilTiler(); -var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded', 'loadedNotes'); +var dispatch = d3_dispatch('apiStatusChange', 'authLoading', 'authDone', 'change', 'loading', 'loaded', 'loadedNotes'); var urlroot = 'https://www.openstreetmap.org'; var oauth = osmAuth({ url: urlroot, @@ -27,6 +27,7 @@ var _blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*']; var _tileCache = { toLoad: {}, loaded: {}, inflight: {}, seen: {}, rtree: new RBush() }; var _noteCache = { toLoad: {}, loaded: {}, inflight: {}, inflightPost: {}, note: {}, closed: {}, rtree: new RBush() }; var _userCache = { toLoad: {}, user: {} }; +var _cachedApiStatus; var _changeset = {}; var _deferred = new Set(); @@ -383,6 +384,7 @@ export default { _tileCache = { toLoad: {}, loaded: {}, inflight: {}, seen: {}, rtree: new RBush() }; _noteCache = { toLoad: {}, loaded: {}, inflight: {}, inflightPost: {}, note: {}, closed: {}, rtree: new RBush() }; _userCache = { toLoad: {}, user: {} }; + _cachedApiStatus = undefined; _changeset = {}; return this; @@ -463,6 +465,13 @@ export default { (err.status === 509 || err.status === 429)) { _rateLimitError = err; dispatch.call('change'); + that.reloadApiStatus(); + + } else if ((err && _cachedApiStatus === 'online') || + (!err && _cachedApiStatus !== 'online')) { + // If the response's error state doesn't match the status, + // it's likely we lost or gained the connection so reload the status + that.reloadApiStatus(); } if (callback) { @@ -778,7 +787,10 @@ export default { .catch(function(err) { errback(err.message); }); function done(err, xml) { - if (err) { return callback(err); } + if (err) { + // the status is null if no response could be retrieved + return callback(err, null); + } // update blacklists var elements = xml.getElementsByTagName('blacklist'); @@ -803,6 +815,24 @@ export default { } }, + // Calls `status` and dispatches an `apiStatusChange` event if the returned + // status differs from the cached status. + reloadApiStatus: function() { + // throttle to avoid unncessary API calls + if (!this.throttledReloadApiStatus) { + var that = this; + this.throttledReloadApiStatus = _throttle(function() { + that.status(function(err, status) { + if (status !== _cachedApiStatus) { + _cachedApiStatus = status; + dispatch.call('apiStatusChange', that, err, status); + } + }); + }, 500); + } + this.throttledReloadApiStatus(); + }, + // Load data (entities) from the API in tiles // GET /api/0.6/map?bbox= diff --git a/modules/ui/status.js b/modules/ui/status.js index 2a98cdbf0..2a2cf6f58 100644 --- a/modules/ui/status.js +++ b/modules/ui/status.js @@ -1,3 +1,4 @@ +import _throttle from 'lodash-es/throttle'; import { event as d3_event } from 'd3-selection'; import { t } from '../util/locale'; @@ -11,48 +12,68 @@ export function uiStatus(context) { return function(selection) { if (!osm) return; - function update() { - osm.status(function(err, apiStatus) { - selection.html(''); + function update(err, apiStatus) { + selection.html(''); - if (err) { - if (apiStatus === 'connectionSwitched') { - // if the connection was just switched, we can't rely on - // the status (we're getting the status of the previous api) - return; + if (err) { + if (apiStatus === 'connectionSwitched') { + // if the connection was just switched, we can't rely on + // the status (we're getting the status of the previous api) + return; - } else if (apiStatus === 'rateLimited') { - selection - .text(t('status.rateLimit')) - .append('a') - .attr('class', 'api-status-login') - .attr('target', '_blank') - .call(svgIcon('#iD-icon-out-link', 'inline')) - .append('span') - .text(t('login')) - .on('click.login', function() { - d3_event.preventDefault(); - osm.authenticate(); - }); - } else { - // eslint-disable-next-line no-warning-comments - // TODO: nice messages for different error types - selection.text(t('status.error')); - } + } else if (apiStatus === 'rateLimited') { + selection + .text(t('osm_api_status.message.rateLimit')) + .append('a') + .attr('class', 'api-status-login') + .attr('target', '_blank') + .call(svgIcon('#iD-icon-out-link', 'inline')) + .append('span') + .text(t('login')) + .on('click.login', function() { + d3_event.preventDefault(); + osm.authenticate(); + }); + } else { - } else if (apiStatus === 'readonly') { - selection.text(t('status.readonly')); - } else if (apiStatus === 'offline') { - selection.text(t('status.offline')); + // don't allow retrying too rapidly + var throttledRetry = _throttle(function() { + // try loading the visible tiles + context.loadTiles(context.projection); + // manually reload the status too in case all visible tiles were already loaded + osm.reloadApiStatus(); + }, 2000); + + // eslint-disable-next-line no-warning-comments + // TODO: nice messages for different error types + selection + .text(t('osm_api_status.message.error') + ' ') + .append('a') + // let the user manually retry their connection directly + .text(t('osm_api_status.retry')) + .on('click.retry', function() { + d3_event.preventDefault(); + throttledRetry(); + }); } - selection.attr('class', 'api-status ' + (err ? 'error' : apiStatus)); - }); + } else if (apiStatus === 'readonly') { + selection.text(t('osm_api_status.message.readonly')); + } else if (apiStatus === 'offline') { + selection.text(t('osm_api_status.message.offline')); + } + + selection.attr('class', 'api-status ' + (err ? 'error' : apiStatus)); } - osm.on('change', function() { update(selection); }); + osm.on('apiStatusChange.uiStatus', update); - window.setInterval(update, 90000); - update(selection); + // reload the status periodically regardless of other factors + window.setInterval(function() { + osm.reloadApiStatus(); + }, 90000); + + // load the initial status in case no OSM data was loaded yet + osm.reloadApiStatus(); }; }