From 5da06645b031e27b18188a8fddffc33126a617e0 Mon Sep 17 00:00:00 2001 From: John Cowen Date: Wed, 22 Sep 2021 18:26:36 +0100 Subject: [PATCH] ui: Gracefully recover from non-existent DC errors (#11077) * ui: Gracefully recover from non-existent DC errors This PR fixes what happens in the UI if you try to navigate to a non-existing DC. When we received a 500 error from an API response due to a non-existent DC, previously we would show a 404 error, which is what we were trying to convey. But in the spirit of the UI being a 'thin client', its probably best to just show the 500 error from the API response, which may help folks to debug any issues better. * Automatically set the CONSUL_DATACENTER_LOCAL env var for testing --- .changelog/11077.txt | 3 ++ .../consul-ui/app/helpers/cached-model.js | 11 +++++ .../consul-ui/app/services/repository.js | 7 +++ .../consul-ui/app/templates/application.hbs | 45 +++++++++++++------ .../consul-ui/mock-api/v1/catalog/datacenters | 2 +- .../tests/acceptance/dc/error.feature | 2 - .../acceptance/dc/services/error.feature | 7 ++- .../consul-ui/tests/steps/doubles/model.js | 6 +++ 8 files changed, 63 insertions(+), 20 deletions(-) create mode 100644 .changelog/11077.txt create mode 100644 ui/packages/consul-ui/app/helpers/cached-model.js diff --git a/.changelog/11077.txt b/.changelog/11077.txt new file mode 100644 index 000000000..9cc2a4fe3 --- /dev/null +++ b/.changelog/11077.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: Gracefully recover from non-existant DC errors +``` diff --git a/ui/packages/consul-ui/app/helpers/cached-model.js b/ui/packages/consul-ui/app/helpers/cached-model.js new file mode 100644 index 000000000..ab2f2ee16 --- /dev/null +++ b/ui/packages/consul-ui/app/helpers/cached-model.js @@ -0,0 +1,11 @@ +import Helper from '@ember/component/helper'; +import { getOwner } from '@ember/application'; + +export default class CachedHelper extends Helper { + compute([model], hash) { + return () => { + const container = getOwner(this); + return container.lookup(`service:repository/${model}`).cached(hash); + }; + } +} diff --git a/ui/packages/consul-ui/app/services/repository.js b/ui/packages/consul-ui/app/services/repository.js index 75792898d..4758c7f1d 100644 --- a/ui/packages/consul-ui/app/services/repository.js +++ b/ui/packages/consul-ui/app/services/repository.js @@ -99,6 +99,13 @@ export default class RepositoryService extends Service { return this.store.peekRecord(this.getModelName(), id); } + cached(params) { + const entries = Object.entries(params); + return this.store.peekAll(this.getModelName()).filter(item => { + return entries.every(([key, value]) => item[key] === value); + }); + } + // @deprecated findAllByDatacenter(params, configuration = {}) { return this.findAll(...arguments); diff --git a/ui/packages/consul-ui/app/templates/application.hbs b/ui/packages/consul-ui/app/templates/application.hbs index f902d58bb..a732cffd9 100644 --- a/ui/packages/consul-ui/app/templates/application.hbs +++ b/ui/packages/consul-ui/app/templates/application.hbs @@ -58,24 +58,44 @@ as |source|> {{! Make sure we guess and default to the right params when not found }} {{#let - (or notfound.dc route.params.dc (env "CONSUL_DATACENTER_LOCAL")) (if (can "use partitions") (or route.params.partition notfound.partition token.Partition 'default') 'default') (if (can "use nspaces") (or route.params.nspace notfound.nspace token.Namespace 'default') 'default') -as |dc partition nspace|}} +as |partition nspace|}} {{! Make sure we have enough to show the app chrome}} -{{!FIXME}} -{{#if (gt dc.length 0)}} - - {{! Don't show anything until we have a list of DCs }} - +{{! Don't show anything until we have a list of DCs }} + +{{! Once we have a list of DCs make sure the DC we are asking for exists }} +{{! If not use the DC that the UI is running in }} {{#let + (or + + (get (object-at 0 (cached-model + 'dc' + (hash + Name=notfound.dc + ) + )) 'Name') + + (get (object-at 0 (cached-model + 'dc' + (hash + Name=route.params.dc + ) + )) 'Name') + + (env "CONSUL_DATACENTER_LOCAL") + + ) + dcs.data -as |dcs|}} - {{#if (and dcs nspace partition)}} + +as |dc dcs|}} + + {{#if (and (gt dc.length 0) dcs nspace partition)}} {{! figure out our current DC and convert it to a model }} {{/if}} {{/let}} - -{{/if}} + {{/let}} {{/if}} diff --git a/ui/packages/consul-ui/mock-api/v1/catalog/datacenters b/ui/packages/consul-ui/mock-api/v1/catalog/datacenters index 1b091390c..5cfa3ee6b 100644 --- a/ui/packages/consul-ui/mock-api/v1/catalog/datacenters +++ b/ui/packages/consul-ui/mock-api/v1/catalog/datacenters @@ -2,7 +2,7 @@ ${ range(env('CONSUL_DATACENTER_COUNT', 10)).map((item, i) => { if(i === 0) { - return `"dc1"`; + return `"${env('CONSUL_DATACENTER_LOCAL', 'dc1')}"`; } return ` "${fake.address.countryCode().toLowerCase()}_${ i % 2 ? "west" : "east"}-${i}" diff --git a/ui/packages/consul-ui/tests/acceptance/dc/error.feature b/ui/packages/consul-ui/tests/acceptance/dc/error.feature index 556c9a44b..89180bbb4 100644 --- a/ui/packages/consul-ui/tests/acceptance/dc/error.feature +++ b/ui/packages/consul-ui/tests/acceptance/dc/error.feature @@ -23,8 +23,6 @@ Feature: dc / error: Recovering from a dc 500 error Then the url should be /dc-500/services And the title should be "Consul" Then I see status on the error like "500" - # FIXME - @ignore Scenario: Clicking the back to root button Given the url "/v1/internal/ui/services" responds with a 200 status When I click home diff --git a/ui/packages/consul-ui/tests/acceptance/dc/services/error.feature b/ui/packages/consul-ui/tests/acceptance/dc/services/error.feature index 09df60e6b..8c3368bf1 100644 --- a/ui/packages/consul-ui/tests/acceptance/dc/services/error.feature +++ b/ui/packages/consul-ui/tests/acceptance/dc/services/error.feature @@ -1,6 +1,4 @@ @setupApplicationTest -# FIXME -@ignore Feature: dc / services / error Scenario: Arriving at the service page that doesn't exist Given 2 datacenter models from yaml @@ -8,11 +6,12 @@ Feature: dc / services / error - dc-1 - dc-2 --- + Given the url "/v1/internal/ui/services" responds with a 500 status When I visit the services page for yaml --- - dc: 404-datacenter + dc: non-existent-datacenter --- - Then I see status on the error like "404" + Then I see status on the error like "500" @notNamespaceable Scenario: Arriving at the service page Given 2 datacenter models from yaml diff --git a/ui/packages/consul-ui/tests/steps/doubles/model.js b/ui/packages/consul-ui/tests/steps/doubles/model.js index 875e9060f..68277239c 100644 --- a/ui/packages/consul-ui/tests/steps/doubles/model.js +++ b/ui/packages/consul-ui/tests/steps/doubles/model.js @@ -7,11 +7,17 @@ export default function(scenario, create, set, win = window, doc = document) { return create(number, model); }) .given(['$number $model model[s]? with the value "$value"'], function(number, model, value) { + if (model === 'dc') { + doc.cookie = `CONSUL_DATACENTER_LOCAL=${value}`; + } return create(number, model, value); }) .given( ['$number $model model[s]? from yaml\n$yaml', '$number $model model[s]? from json\n$json'], function(number, model, data) { + if (model === 'dc') { + doc.cookie = `CONSUL_DATACENTER_LOCAL=${data[0]}`; + } return create(number, model, data); } )