Invalidate session before loading

This fixes a bug where a failed `viewer.load()` call would poison the
viewer and prevent it from loading any more tables.

Signed-off-by: Tom Jakubowski <[email protected]>

Author: tomjakubowski

packages/perspective-workspace/test/js/table.spec.js

@@ -92,7 +92,9 @@ function tests(context, compare) {
 
         // NOTE This is the error message we expect when `restore()` is called
         // without a `Table`, subject to change.
-        expect(result).toEqual("Error: `restore()` called before `load()`");
+        expect(result).toEqual(
+            "Error: Trying to draw the viewer with no table attached"
+        );
         await page.evaluate(async () => {
             await workspace.replaceTable(
                 "errored",

rust/perspective-viewer/src/rust/components/error_message.rs

@@ -38,7 +38,6 @@ pub fn error_message(p: &ErrorMessageProps) -> yew::Html {
             || drop(sub)
         },
     );
-    tracing::error!("Fucked");
     html! {
         <>
             <LocalStyle href={css!("render-warning")} />

rust/perspective-viewer/src/rust/custom_elements/viewer.rs

@@ -154,6 +154,7 @@ impl PerspectiveViewerElement {
     /// ```
     pub fn load(&self, table: JsValue) -> ApiFuture<()> {
         tracing::info!("Loading Table");
+        self.session.invalidate();
         let promise = table
             .clone()
             .dyn_into::<js_sys::Promise>()

rust/perspective-viewer/src/rust/session.rs

@@ -580,7 +580,7 @@ impl Session {
             .borrow()
             .table
             .as_ref()
-            .ok_or("`restore()` called before `load()`")?
+            .ok_or("Trying to draw the viewer with no table attached")?
             .clone();
 
         let valid_recs = table.validate_expressions(config.expressions).await?;

rust/perspective-viewer/test/js/errors.spec.js

@@ -0,0 +1,62 @@
+// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
+// ┃ ██████ ██████ ██████       █      █      █      █      █ █▄  ▀███ █       ┃
+// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█  ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄  ▀█ █ ▀▀▀▀▀ ┃
+// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄   █ ▄▄▄▄▄ ┃
+// ┃ █      ██████ █  ▀█▄       █ ██████      █      ███▌▐███ ███████▄ █       ┃
+// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
+// ┃ Copyright (c) 2017, the Perspective Authors.                              ┃
+// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃
+// ┃ This file is part of the Perspective library, distributed under the terms ┃
+// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
+// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
+
+import { test, expect } from "@finos/perspective-test";
+
+test.beforeEach(async ({ page }) => {
+    await page.goto("/rust/perspective-viewer/test/html/blank.html");
+    await page.waitForFunction(() => "WORKER" in window);
+});
+
+test.describe("viewer.load() method", async () => {
+    test("does load a resolved Table promise", async ({ page }) => {
+        const viewer = page.locator("perspective-viewer");
+        await viewer.evaluate(async (viewer) => {
+            const goodTable = (await window.WORKER).table("a,b,c\n1,2,3");
+            return viewer.load(goodTable);
+        });
+        await expect(viewer).toHaveText(/"a","b","c"/); // column titles
+    });
+
+    test("is rejected by a rejected Table promise", async ({ page }) => {
+        const viewer = page.locator("perspective-viewer");
+        await expect(
+            viewer.evaluate((viewer) => {
+                const errorTable = Promise.reject(new Error("blimpy"));
+                return viewer.load(errorTable);
+            })
+        ).rejects.toThrow("blimpy");
+    });
+
+    test("after a load error, same viewer can load a resolved Table promise", async ({
+        page,
+    }) => {
+        const viewer = page.locator("perspective-viewer");
+        const didError = await viewer.evaluate(async (viewer) => {
+            const errorTable = Promise.reject(new Error("blimpy"));
+            const worker = await window.WORKER;
+            let didError = false;
+            try {
+                await viewer.load(errorTable);
+            } catch (e) {
+                if (e.message.includes("blimpy")) {
+                    didError = true;
+                }
+            }
+
+            const goodTable = worker.table("a,b,c\n1,2,3");
+            await viewer.load(goodTable);
+            return didError;
+        });
+        expect(didError).toBe(true);
+    });
+});

rust/perspective-viewer/test/js/settings.spec.js

@@ -111,11 +111,11 @@ test.describe("Settings", () => {
 
             consoleLogs.expectedLogs.push(
                 "error",
-                /Failed to apply config: Error: `restore\(\)` called before `load\(\)`.*/
+                /Failed to apply config: Error: Trying to draw the viewer with no table attached/
             );
             consoleLogs.expectedLogs.push(
                 "error",
-                /Caught error: Error: `restore\(\)` called before `load\(\)`.*/
+                /Caught error: Error: Trying to draw the viewer with no table attached/
             );
         });
     });

tools/perspective-scripts/test_js.mjs

@@ -76,8 +76,8 @@ function playwright(pkg, is_jlab) {
 
     return sh`
         TZ=UTC
-        npx playwright test 
-        --config=tools/perspective-test/playwright.config.ts 
+        npx playwright test
+        --config=tools/perspective-test/playwright.config.ts
         ${args}
     `.env(env);
 }