From f4208b5fba1b7994c77cc64be31471e4d4df8a18 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 27 Oct 2020 14:51:15 +0000 Subject: [PATCH] ui: Metrics - Don't swallow metrics errors (#9044) * ui: Make eventsources use http-like errors for stopping * ui: Don't swallow errors from prometheus, pass them to the ui to handle --- .../app/utils/dom/event-source/blocking.js | 5 ++- .../utils/dom/event-source/blocking-test.js | 4 +- .../vendor/metrics-providers/prometheus.js | 38 ++++++------------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/ui/packages/consul-ui/app/utils/dom/event-source/blocking.js b/ui/packages/consul-ui/app/utils/dom/event-source/blocking.js index e0d9ef5cb2..8ff1ad539e 100644 --- a/ui/packages/consul-ui/app/utils/dom/event-source/blocking.js +++ b/ui/packages/consul-ui/app/utils/dom/event-source/blocking.js @@ -7,8 +7,11 @@ const pause = 2000; 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'); + // expect and ember-data error or a http-like error (e.statusCode) + let status = get(err, 'errors.firstObject.status') || get(err, 'statusCode'); if (typeof status !== 'undefined') { + // ember-data errors are strings, http errors are numbers + status = status.toString(); switch (true) { // Any '5xx' (not 500) errors should back off and try again case status.indexOf('5') === 0 && status.length === 3 && status !== '500': diff --git a/ui/packages/consul-ui/tests/unit/utils/dom/event-source/blocking-test.js b/ui/packages/consul-ui/tests/unit/utils/dom/event-source/blocking-test.js index 51a0e3d212..49d6dac984 100644 --- a/ui/packages/consul-ui/tests/unit/utils/dom/event-source/blocking-test.js +++ b/ui/packages/consul-ui/tests/unit/utils/dom/event-source/blocking-test.js @@ -51,8 +51,8 @@ module('Unit | Utility | dom/event-source/blocking', function() { undefined, null, new Error(), + { statusCode: 404 }, { errors: [] }, - { errors: [{ status: 501 }] }, { errors: [{ status: '401' }] }, { errors: [{ status: '500' }] }, { errors: [{ status: '5' }] }, @@ -67,6 +67,8 @@ module('Unit | Utility | dom/event-source/blocking', function() { }); test('the 5xx backoff returns a resolve promise on a 5xx (apart from 500)', function(assert) { [ + { statusCode: 501 }, + { errors: [{ status: 501 }] }, { errors: [{ status: '501' }] }, { errors: [{ status: '503' }] }, { errors: [{ status: '504' }] }, diff --git a/ui/packages/consul-ui/vendor/metrics-providers/prometheus.js b/ui/packages/consul-ui/vendor/metrics-providers/prometheus.js index 151dea6a27..52becc2918 100644 --- a/ui/packages/consul-ui/vendor/metrics-providers/prometheus.js +++ b/ui/packages/consul-ui/vendor/metrics-providers/prometheus.js @@ -255,7 +255,7 @@ }, fetchStats: function(statsPromises) { - var all = Promise.allSettled(statsPromises). + var all = Promise.all(statsPromises). then(function(results){ var data = { stats: [] @@ -263,9 +263,7 @@ // Add all non-empty stats for (var i = 0; i < statsPromises.length; i++) { if (results[i].value) { - data.stats.push(results[i].value); - } else if (results[i].reason) { - console.log("ERROR processing stat", results[i].reason) + data.stats.push(results[i]); } } return data @@ -276,23 +274,21 @@ }, fetchStatsGrouped: function(statsPromises) { - var all = Promise.allSettled(statsPromises). + var all = Promise.all(statsPromises). then(function(results){ var data = { stats: {} } // Add all non-empty stats for (var i = 0; i < statsPromises.length; i++) { - if (results[i].value) { - for (var group in results[i].value) { - if (!results[i].value.hasOwnProperty(group)) continue; + if (results[i]) { + for (var group in results[i]) { + if (!results[i].hasOwnProperty(group)) continue; if (!data.stats[group]) { data.stats[group] = [] } - data.stats[group].push(results[i].value[group]) + data.stats[group].push(results[i][group]) } - } else if (results[i].reason) { - console.log("ERROR processing stat", results[i].reason) } } return data @@ -363,11 +359,7 @@ Errors: 'Error responses (with an HTTP response code in the 5xx range) per second.', }; return this.fetchSeries(q, options) - .then(this.reformatSeries(" rps", labelMap), function(xhr){ - // Failure. log to console and return a blank result for now. - console.log('ERROR: failed to fetch requestRate', xhr.responseText) - return emptySeries; - }) + .then(this.reformatSeries(" rps", labelMap)) }, fetchDataRateSeries: function(serviceName, options){ @@ -394,11 +386,7 @@ Outbound: 'Outbound data rate (data transmitted) from the network in bits per second.', }; return this.fetchSeries(q, options) - .then(this.reformatSeries("bps", labelMap), function(xhr){ - // Failure. log to console and return a blank result for now. - console.log('ERROR: failed to fetch requestRate', xhr.responseText) - return emptySeries; - }) + .then(this.reformatSeries("bps", labelMap)) }, makeSubject: function(serviceName, type) { @@ -654,10 +642,6 @@ } } return data; - }, function(xhr){ - // Failure. log to console and return an blank result for now. - console.log("ERROR: failed to fetch stat", label, xhr.responseText) - return {} }) }, @@ -684,7 +668,9 @@ var o = JSON.parse(xhr.responseText) resolve(o) } - reject(xhr) + const e = new Error(xhr.statusText); + e.statusCode = xhr.status; + reject(e); } var url = self.baseURL()+path;