Add notification in renderer if lens protocol handler fails to find any routes (#2787)

- Add notifications on missing lens:// handlers, and invalid URIs

- add notifications on unknown entity IDs

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/main/index.ts

@@ -93,8 +93,11 @@ if (!app.requestSingleInstanceLock()) {
 
   for (const arg of process.argv) {
     if (arg.toLowerCase().startsWith("lens://")) {
-      lprm.route(arg)
-        .catch(error => logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl: arg }));
+      try {
+        lprm.route(arg);
+      } catch (error) {
+        logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl: arg });
+      }
     }
   }
 }
@@ -104,8 +107,11 @@ app.on("second-instance", (event, argv) => {
 
   for (const arg of argv) {
     if (arg.toLowerCase().startsWith("lens://")) {
-      lprm.route(arg)
-        .catch(error => logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl: arg }));
+      try {
+        lprm.route(arg);
+      } catch (error) {
+        logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl: arg });
+      }
     }
   }
 
@@ -252,9 +258,10 @@ autoUpdater.on("before-quit-for-update", () => blockQuit = false);
 app.on("will-quit", (event) => {
   // Quit app on Cmd+Q (MacOS)
   logger.info("APP:QUIT");
-  appEventBus.emit({name: "app", action: "close"});
+  appEventBus.emit({ name: "app", action: "close" });
   ClusterManager.getInstance(false)?.stop(); // close cluster connections
   KubeconfigSyncManager.getInstance(false)?.stopSync();
+  LensProtocolRouterMain.getInstance(false)?.cleanup();
   cleanup();
 
   if (blockQuit) {
@@ -268,10 +275,11 @@ app.on("open-url", (event, rawUrl) => {
   // lens:// protocol handler
   event.preventDefault();
 
-  LensProtocolRouterMain
-    .getInstance()
-    .route(rawUrl)
-    .catch(error => logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl }));
+  try {
+    LensProtocolRouterMain.getInstance().route(rawUrl);
+  } catch (error) {
+    logger.error(`${LensProtocolRouterMain.LoggingPrefix}: an error occured`, { error, rawUrl });
+  }
 });
 
 /**

src/main/protocol-handler/__test__/router.test.ts

@@ -23,7 +23,7 @@ import * as uuid from "uuid";
 
 import { broadcastMessage } from "../../../common/ipc";
 import { ProtocolHandlerExtension, ProtocolHandlerInternal } from "../../../common/protocol-handler";
-import { noop } from "../../../common/utils";
+import { delay, noop } from "../../../common/utils";
 import { LensExtension } from "../../../extensions/main-api";
 import { ExtensionLoader } from "../../../extensions/extension-loader";
 import { ExtensionsStore } from "../../../extensions/extensions-store";
@@ -56,27 +56,27 @@ describe("protocol router tests", () => {
     LensProtocolRouterMain.resetInstance();
   });
 
-  it("should throw on non-lens URLS", async () => {
+  it("should throw on non-lens URLS", () => {
     try {
       const lpr = LensProtocolRouterMain.getInstance();
 
-      expect(await lpr.route("https://google.ca")).toBeUndefined();
+      expect(lpr.route("https://google.ca")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
   });
 
-  it("should throw when host not internal or extension", async () => {
+  it("should throw when host not internal or extension", () => {
     try {
       const lpr = LensProtocolRouterMain.getInstance();
 
-      expect(await lpr.route("lens://foobar")).toBeUndefined();
+      expect(lpr.route("lens://foobar")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
   });
 
-  it.only("should not throw when has valid host", async () => {
+  it("should not throw when has valid host", async () => {
     const extId = uuid.v4();
     const ext = new LensExtension({
       id: extId,
@@ -102,38 +102,39 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/", noop);
 
     try {
-      expect(await lpr.route("lens://app")).toBeUndefined();
+      expect(lpr.route("lens://app")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     try {
-      expect(await lpr.route("lens://extension/@mirantis/minikube")).toBeUndefined();
+      expect(lpr.route("lens://extension/@mirantis/minikube")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
-    expect(broadcastMessage).toHaveBeenNthCalledWith(1, ProtocolHandlerInternal, "lens://app/");
-    expect(broadcastMessage).toHaveBeenNthCalledWith(2, ProtocolHandlerExtension, "lens://extension/@mirantis/minikube");
+    await delay(50);
+    expect(broadcastMessage).toHaveBeenCalledWith(ProtocolHandlerInternal, "lens://app/", "matched");
+    expect(broadcastMessage).toHaveBeenCalledWith(ProtocolHandlerExtension, "lens://extension/@mirantis/minikube", "matched");
   });
 
-  it("should call handler if matches", async () => {
+  it("should call handler if matches", () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called = false;
 
     lpr.addInternalHandler("/page", () => { called = true; });
 
     try {
-      expect(await lpr.route("lens://app/page")).toBeUndefined();
+      expect(lpr.route("lens://app/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     expect(called).toBe(true);
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page", "matched");
   });
 
-  it("should call most exact handler", async () => {
+  it("should call most exact handler", () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -141,13 +142,13 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/:id", params => { called = params.pathname.id; });
 
     try {
-      expect(await lpr.route("lens://app/page/foo")).toBeUndefined();
+      expect(lpr.route("lens://app/page/foo")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     expect(called).toBe("foo");
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo", "matched");
   });
 
   it("should call most exact handler for an extension", async () => {
@@ -180,13 +181,14 @@ describe("protocol router tests", () => {
     (ExtensionsStore.getInstance() as any).state.set(extId, { enabled: true, name: "@foobar/icecream" });
 
     try {
-      expect(await lpr.route("lens://extension/@foobar/icecream/page/foob")).toBeUndefined();
+      expect(lpr.route("lens://extension/@foobar/icecream/page/foob")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
+    await delay(50);
     expect(called).toBe("foob");
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, "lens://extension/@foobar/icecream/page/foob");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, "lens://extension/@foobar/icecream/page/foob", "matched");
   });
 
   it("should work with non-org extensions", async () => {
@@ -245,13 +247,15 @@ describe("protocol router tests", () => {
     (ExtensionsStore.getInstance() as any).state.set("icecream", { enabled: true, name: "icecream" });
 
     try {
-      expect(await lpr.route("lens://extension/icecream/page")).toBeUndefined();
+      expect(lpr.route("lens://extension/icecream/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
+    await delay(50);
+
     expect(called).toBe(1);
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, "lens://extension/icecream/page");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, "lens://extension/icecream/page", "matched");
   });
 
   it("should throw if urlSchema is invalid", () => {
@@ -260,7 +264,7 @@ describe("protocol router tests", () => {
     expect(() => lpr.addInternalHandler("/:@", noop)).toThrowError();
   });
 
-  it("should call most exact handler with 3 found handlers", async () => {
+  it("should call most exact handler with 3 found handlers", () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -270,16 +274,16 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(await lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
+      expect(lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     expect(called).toBe(3);
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo/bar/bat");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo/bar/bat", "matched");
   });
 
-  it("should call most exact handler with 2 found handlers", async () => {
+  it("should call most exact handler with 2 found handlers", () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -288,12 +292,12 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(await lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
+      expect(lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     expect(called).toBe(1);
-    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo/bar/bat");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo/bar/bat", "matched");
   });
 });

src/main/protocol-handler/router.ts

@@ -25,6 +25,8 @@ import Url from "url-parse";
 import type { LensExtension } from "../../extensions/lens-extension";
 import { broadcastMessage } from "../../common/ipc";
 import { observable, when, makeObservable } from "mobx";
+import { ProtocolHandlerInvalid, RouteAttempt } from "../../common/protocol-handler";
+import { disposer } from "../../common/utils";
 
 export interface FallbackHandler {
   (name: string): Promise<boolean>;
@@ -36,19 +38,25 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
   @observable rendererLoaded = false;
   @observable extensionsLoaded = false;
 
+  protected disposers = disposer();
+
   constructor() {
     super();
 
     makeObservable(this);
   }
 
+  public cleanup() {
+    this.disposers();
+  }
+
   /**
    * Find the most specific registered handler, if it exists, and invoke it.
    *
    * This will send an IPC message to the renderer router to do the same
    * in the renderer.
    */
-  public async route(rawUrl: string): Promise<void> {
+  public route(rawUrl: string) {
     try {
       const url = new Url(rawUrl, true);
 
@@ -60,16 +68,18 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
 
       switch (url.host) {
         case "app":
-          return this._routeToInternal(url);
+          this._routeToInternal(url);
+          break;
         case "extension":
-          await when(() => this.extensionsLoaded);
-
-          return this._routeToExtension(url);
+          this.disposers.push(when(() => this.extensionsLoaded, () => this._routeToExtension(url)));
+          break;
         default:
           throw new proto.RoutingError(proto.RoutingErrorType.INVALID_HOST, url);
       }
 
     } catch (error) {
+      broadcastMessage(ProtocolHandlerInvalid, error.toString(), rawUrl);
+
       if (error instanceof proto.RoutingError) {
         logger.error(`${proto.LensProtocolRouter.LoggingPrefix}: ${error}`, { url: error.url });
       } else {
@@ -102,17 +112,16 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
     return "";
   }
 
-  protected async _routeToInternal(url: Url): Promise<void> {
+  protected _routeToInternal(url: Url): RouteAttempt {
     const rawUrl = url.toString(); // for sending to renderer
+    const attempt = super._routeToInternal(url);
 
-    super._routeToInternal(url);
+    this.disposers.push(when(() => this.rendererLoaded, () => broadcastMessage(proto.ProtocolHandlerInternal, rawUrl, attempt)));
 
-    await when(() => this.rendererLoaded);
-
-    return broadcastMessage(proto.ProtocolHandlerInternal, rawUrl);
+    return attempt;
   }
 
-  protected async _routeToExtension(url: Url): Promise<void> {
+  protected async _routeToExtension(url: Url): Promise<RouteAttempt> {
     const rawUrl = url.toString(); // for sending to renderer
 
     /**
@@ -122,10 +131,11 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
      * Note: this needs to clone the url because _routeToExtension modifies its
      * argument.
      */
-    await super._routeToExtension(new Url(url.toString(), true));
-    await when(() => this.rendererLoaded);
+    const attempt = await super._routeToExtension(new Url(url.toString(), true));
 
-    return broadcastMessage(proto.ProtocolHandlerExtension, rawUrl);
+    this.disposers.push(when(() => this.rendererLoaded, () => broadcastMessage(proto.ProtocolHandlerExtension, rawUrl, attempt)));
+
+    return attempt;
   }
 
   /**