From 22d60ab4368bae4f11bfa2fe5b93abaf532c2c0e Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 11 Aug 2020 18:47:15 +0100 Subject: [PATCH] ui: Reduce reconnection attempts on disconnection (#8481) * ui: Reduce reconnection attempts on disconnection The UI will attempt to reconnect/retry a blocking query to Consul after a disconnection in certain circumstances. 1. On receipt of a 5xx error (used for keeping blocking queries running through reverse proxies that have lowertimeouts than consul itself) 2. When a user switches to a different tab and back again) 3. When the connection to Consul is dropped entirely (when Consul itself has exited) In the last case the retry attempts where not using a 3 second interval between attempts like the first case is. This commit changes the last case to use the same 3 second pause as the last case. --- ui-v2/app/services/client/connections.js | 2 +- ui-v2/app/services/client/http.js | 4 +++- ui-v2/app/utils/dom/event-source/blocking.js | 13 ++++++++++--- .../unit/utils/dom/event-source/blocking-test.js | 7 +++---- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/ui-v2/app/services/client/connections.js b/ui-v2/app/services/client/connections.js index e5ca38cd6a..32f11214dd 100644 --- a/ui-v2/app/services/client/connections.js +++ b/ui-v2/app/services/client/connections.js @@ -21,7 +21,7 @@ export default Service.extend({ this._listeners.add(this.dom.document(), { visibilitychange: e => { if (e.target.hidden) { - this.purge(); + this.purge(-1); } }, }); diff --git a/ui-v2/app/services/client/http.js b/ui-v2/app/services/client/http.js index a9525afb3c..64c8925806 100644 --- a/ui-v2/app/services/client/http.js +++ b/ui-v2/app/services/client/http.js @@ -15,7 +15,9 @@ export const restartWhenAvailable = function(client) { // setup the aborted connection restarting // this should happen here to avoid cache deletion const status = get(e, 'errors.firstObject.status'); - if (status === '0') { + // TODO: Reconsider a proper custom HTTP code + // -1 is a UI only error code for 'user switched tab' + if (status === '-1') { // Any '0' errors (abort) should possibly try again, depending upon the circumstances // whenAvailable returns a Promise that resolves when the client is available // again diff --git a/ui-v2/app/utils/dom/event-source/blocking.js b/ui-v2/app/utils/dom/event-source/blocking.js index 767275883a..fb68934c5f 100644 --- a/ui-v2/app/utils/dom/event-source/blocking.js +++ b/ui-v2/app/utils/dom/event-source/blocking.js @@ -2,7 +2,9 @@ import { get } from '@ember/object'; const pause = 2000; // native EventSource retry is ~3s wait -export const create5xxBackoff = function(ms = 3000, P = Promise, wait = setTimeout) { +// any specified errors here will mean that the blocking query will attempt +// a reconnection every 3s until it reconnects to Consul +export const createErrorBackoff = function(ms = 3000, P = Promise, wait = setTimeout) { // This expects an ember-data like error return function(err) { const status = get(err, 'errors.firstObject.status'); @@ -10,6 +12,11 @@ export const create5xxBackoff = function(ms = 3000, P = Promise, wait = setTimeo switch (true) { // Any '5xx' (not 500) errors should back off and try again case status.indexOf('5') === 0 && status.length === 3 && status !== '500': + // fallsthrough + case status === '0': + // TODO: Move this to the view layer so we can show a connection error + // and reconnection success to the user + // Any 0 aborted connections should back off and try again return new P(function(resolve) { wait(function() { resolve(err); @@ -51,9 +58,9 @@ const defaultCreateEvent = function(result, configuration) { * Wraps an EventSource with functionality to add native EventSource-like functionality * * @param {Class} [CallableEventSource] - CallableEventSource Class - * @param {Function} [backoff] - Default backoff function for all instances, defaults to create5xxBackoff + * @param {Function} [backoff] - Default backoff function for all instances, defaults to createErrorBackoff */ -export default function(EventSource, backoff = create5xxBackoff()) { +export default function(EventSource, backoff = createErrorBackoff()) { /** * An EventSource implementation to add native EventSource-like functionality with just callbacks (`cursor` and 5xx backoff) * diff --git a/ui-v2/tests/unit/utils/dom/event-source/blocking-test.js b/ui-v2/tests/unit/utils/dom/event-source/blocking-test.js index 65786025c9..51a0e3d212 100644 --- a/ui-v2/tests/unit/utils/dom/event-source/blocking-test.js +++ b/ui-v2/tests/unit/utils/dom/event-source/blocking-test.js @@ -1,6 +1,6 @@ import domEventSourceBlocking, { validateCursor, - create5xxBackoff, + createErrorBackoff, } from 'consul-ui/utils/dom/event-source/blocking'; import { module } from 'qunit'; import test from 'ember-sinon-qunit/test-support/test'; @@ -46,13 +46,12 @@ module('Unit | Utility | dom/event-source/blocking', function() { assert.ok(source instanceof EventSource); }); test("the 5xx backoff continues to throw when it's not a 5xx", function(assert) { - const backoff = create5xxBackoff(); + const backoff = createErrorBackoff(); [ undefined, null, new Error(), { errors: [] }, - { errors: [{ status: '0' }] }, { errors: [{ status: 501 }] }, { errors: [{ status: '401' }] }, { errors: [{ status: '500' }] }, @@ -76,7 +75,7 @@ module('Unit | Utility | dom/event-source/blocking', function() { const timeout = this.stub().callsArg(0); const resolve = this.stub().withArgs(item); const Promise = createPromise(resolve); - const backoff = create5xxBackoff(undefined, Promise, timeout); + const backoff = createErrorBackoff(undefined, Promise, timeout); const promise = backoff(item); assert.ok(promise instanceof Promise, 'a promise was returned'); assert.ok(resolve.calledOnce, 'the promise was resolved with the correct arguments');