working on making sure we show a 404 page with a proper error (#8927)

* working on making sure we show a 404 page with a proper error

* code cleanup + add logic to capture 404s for resource instance details

* add e2e tests

* address PR comments + adjust e2e tests

* cover 404 on cluster for dynamic plugins

* address PR comments

* catching bogus resources on authenticated middleware with redirect to 404 page

* fix lint issue

* address PR comments + fix issue with e2e tests

* Fix l10n
- Ensure error messages doesn't reference 'list' when not on a list page
- The new way the feature works means going to a list with an unknown resource results in the generic message, but this is preferably over the above

* fix e2e tests

---------

Co-authored-by: Alexandre Alves <aalves@Alexandres-MacBook-Pro.local>
Co-authored-by: Alexandre Alves <aalves@Alexandres-MBP.lan>
Co-authored-by: Richard Cox <richard.cox@suse.com>

cypress/e2e/po/pages/not-found-page.po.ts

@@ -0,0 +1,11 @@
+import PagePo from '@/cypress/e2e/po/pages/page.po';
+
+export default class NotFoundPagePo extends PagePo {
+  errorTitle() {
+    return this.self().get('.text-center h1');
+  }
+
+  errorMessage() {
+    return this.self().get('.text-center h2');
+  }
+}

cypress/e2e/tests/navigation/not-found-page.spec.ts

@@ -0,0 +1,47 @@
+import NotFoundPagePo from '@/cypress/e2e/po/pages/not-found-page.po';
+
+describe('Not found page display', () => {
+  beforeEach(() => {
+    cy.login();
+  });
+
+  it('Will show a 404 if we do not have a valid Product id on the route path', () => {
+    const notFound = new NotFoundPagePo('/c/local/bogus-product-id');
+
+    notFound.goTo();
+    notFound.waitForPage();
+
+    notFound.errorTitle().contains('Error');
+    notFound.errorMessage().contains('Product bogus-product-id not found');
+  });
+
+  it('Will show a 404 if we do not have a valid Resource type on the route path', () => {
+    const notFound = new NotFoundPagePo('/c/local/explorer/bogus-resource-type');
+
+    notFound.goTo();
+    notFound.waitForPage();
+
+    notFound.errorTitle().contains('Error');
+    notFound.errorMessage().contains('Resource type bogus-resource-type not found');
+  });
+
+  it('Will show a 404 if we do not have a valid Resource id on the route path', () => {
+    const notFound = new NotFoundPagePo('/c/local/explorer/node/bogus-resource-id');
+
+    notFound.goTo();
+    notFound.waitForPage();
+
+    notFound.errorTitle().contains('Error');
+    notFound.errorMessage().contains('Resource node with id bogus-resource-id not found, unable to display resource details');
+  });
+
+  it('Will show a 404 if we do not have a valid product + resource + resource id', () => {
+    const notFound = new NotFoundPagePo('/c/local/bogus-product-id/bogus-resource/bogus-resource-id');
+
+    notFound.goTo();
+    notFound.waitForPage();
+
+    notFound.errorTitle().contains('Error');
+    notFound.errorMessage().contains('Product bogus-product-id not found');
+  });
+});

pkg/harvester-manager/config/harvester-manager.js

@@ -51,6 +51,8 @@ dynamicPluginLoader.register({
             return harvesterClustersLocation;
           }
         }
+      } else {
+        return harvesterClustersLocation;
       }
     }
   }

shell/assets/translations/en-us.yaml

@@ -197,6 +197,12 @@ nav:
     accountAndKeys: Account & API Keys
     logOut: Log Out
   failWhale:
+    authMiddleware: Auth Middleware
+    clusterNotFound: Cluster { clusterId } not found
+    productNotFound: Product { productNotFound } not found
+    resourceNotFound: Resource type { resource } not found
+    resourceListNotFound: Resource type { resource } not found, unable to display list
+    resourceIdNotFound: Resource { resource } with id { fqid } not found, unable to display resource details
     reload: Reload
     separator: or
 

shell/components/ResourceDetail/index.vue

@@ -193,6 +193,9 @@ export default {
           opt:  { watch: true }
         });
       } catch (e) {
+        if (e.status === 404) {
+          store.dispatch('loadingError', new Error(this.t('nav.failWhale.resourceIdNotFound', { resource, fqid }, true)));
+        }
         liveModel = {};
         notFound = fqid;
       }
@@ -367,18 +370,7 @@ export default {
 </script>
 
 <template>
-  <Loading v-if="$fetchState.pending" />
+  <Loading v-if="$fetchState.pending || notFound" />
-  <div v-else-if="notFound">
-    <IconMessage icon="icon-warning">
-      <template v-slot:message>
-        {{ t('generic.notFound') }}
-        <div>
-          <div>{{ t('generic.type') }}: {{ resource }}</div>
-          <div>{{ t('generic.id') }}: {{ notFound }}</div>
-        </div>
-      </template>
-    </IconMessage>
-  </div>
   <div v-else>
     <Masthead
       v-if="showMasthead"

shell/components/ResourceList/index.vue

@@ -70,7 +70,7 @@ export default {
 
     if ( !this.hasFetch ) {
       if ( !schema ) {
-        store.dispatch('loadingError', new Error(`Type ${ resource } not found, unable to display list`));
+        store.dispatch('loadingError', new Error(this.t('nav.failWhale.resourceListNotFound', { resource }, true)));
 
         return;
       }

shell/middleware/authenticated.js

@@ -4,7 +4,7 @@ import {
   SETUP, TIMED_OUT, UPGRADED, _FLAGGED, _UNFLAG
 } from '@shell/config/query-params';
 import { SETTING } from '@shell/config/settings';
-import { MANAGEMENT, NORMAN, DEFAULT_WORKSPACE } from '@shell/config/types';
+import { MANAGEMENT, NORMAN, DEFAULT_WORKSPACE, WORKLOAD } from '@shell/config/types';
 import { _ALL_IF_AUTHED } from '@shell/plugins/dashboard-store/actions';
 import { applyProducts } from '@shell/store/type-map';
 import { findBy } from '@shell/utils/array';
@@ -26,6 +26,16 @@ const getPackageFromRoute = (route) => {
   return arraySafe.find((m) => !!m.pkg)?.pkg;
 };
 
+const getResourceFromRoute = (to) => {
+  let resource = to.params?.resource;
+
+  if (!resource) {
+    resource = findMeta(to, 'resource');
+  }
+
+  return resource;
+};
+
 let beforeEachSetup = false;
 
 function findMeta(route, key) {
@@ -71,9 +81,18 @@ export function getProductFromRoute(to) {
   return product;
 }
 
-function setProduct(store, to) {
+function setProduct(store, to, redirect) {
   let product = getProductFromRoute(to);
 
+  // since all products are hardcoded as routes (ex: c-local-explorer), if we match the wildcard route it means that the product does not exist
+  if ((product && (!to.matched.length || (to.matched.length && to.matched[0].path === '/c/:cluster/:product'))) ||
+  // if the product grabbed from the route is not registered, then we don't have it!
+  (product && !store.getters['type-map/isProductRegistered'](product))) {
+    store.dispatch('loadingError', new Error(store.getters['i18n/t']('nav.failWhale.productNotFound', { productNotFound: product }, true)));
+
+    return () => redirect(302, '/fail-whale');
+  }
+
   if ( !product ) {
     product = EXPLORER;
   }
@@ -92,6 +111,8 @@ function setProduct(store, to) {
     // There might be management catalog items in it vs cluster.
     store.commit('catalog/reset');
   }
+
+  return false;
 }
 
 export default async function({
@@ -288,12 +309,21 @@ export default async function({
 
     store.app.router.beforeEach((to, from, next) => {
       // NOTE - This beforeEach runs AFTER this middleware. So anything in this middleware that requires it must set it manually
-      setProduct(store, to);
+      const redirected = setProduct(store, to, redirect);
+
+      if (redirected) {
+        return redirected();
+      }
+
       next();
     });
 
     // Call it for the initial pageload
-    setProduct(store, route);
+    const redirected = setProduct(store, route, redirect);
+
+    if (redirected) {
+      return redirected();
+    }
 
     if (process.client) {
       store.app.router.afterEach((to, from) => {
@@ -376,7 +406,12 @@ export default async function({
     // When fleet moves to it's own package this should be moved to pkg onEnter/onLeave
     if ((oldProduct === FLEET_NAME || product === FLEET_NAME) && oldProduct !== product) {
       // See note above for store.app.router.beforeEach, need to setProduct manually, for the moment do this in a targeted way
-      setProduct(store, route);
+      const redirected = setProduct(store, route, redirect);
+
+      if (redirected) {
+        return redirected();
+      }
+
       store.commit('updateWorkspace', {
         value:   store.getters['prefs/get'](WORKSPACE) || DEFAULT_WORKSPACE,
         getters: store.getters
@@ -397,6 +432,16 @@ export default async function({
       })
     ]);
 
+    const resource = getResourceFromRoute(route);
+
+    // if we have resource param, but can't get the schema, it means the resource doesn't exist!
+    // NOTE: workload doesn't have a schema, so let's just bypass this with the identifier
+    if (resource && resource !== WORKLOAD && !store.getters['management/schemaFor'](resource) && !store.getters['rancher/schemaFor'](resource)) {
+      store.dispatch('loadingError', new Error(store.getters['i18n/t']('nav.failWhale.resourceNotFound', { resource }, true)));
+
+      return redirect(302, '/fail-whale');
+    }
+
     if (!clusterId) {
       clusterId = store.getters['defaultClusterId']; // This needs the cluster list, so no parallel
       const isSingleProduct = store.getters['isSingleProduct'];
@@ -422,7 +467,7 @@ export default async function({
       return redirect(302, '/home');
     } else {
       // Sets error 500 if lost connection to API
-      store.commit('setError', { error: e, locationError: new Error('Auth Middleware') });
+      store.commit('setError', { error: e, locationError: new Error(store.getters['i18n/t']('nav.failWhale.authMiddleware')) });
 
       return redirect(302, '/fail-whale');
     }

shell/store/type-map.js

@@ -1361,6 +1361,14 @@ export const getters = {
       return _rowValueGetter(col);
     };
   },
+
+  isProductRegistered(state) {
+    return (productName) => {
+      const prod = state.products.find((p) => p.name === productName);
+
+      return !!prod;
+    };
+  },
 };
 
 export const mutations = {