Skip to content

Commit 658647e

Browse files
committed
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 <tom@prospective.dev>
1 parent ce1a54e commit 658647e

File tree

8 files changed

+71
-7
lines changed

8 files changed

+71
-7
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ function tests(context, compare) {
9292

9393
// NOTE This is the error message we expect when `restore()` is called
9494
// without a `Table`, subject to change.
95-
expect(result).toEqual("Error: `restore()` called before `load()`");
95+
expect(result).toEqual(
96+
"Error: Trying to draw the viewer with no table attached"
97+
);
9698
await page.evaluate(async () => {
9799
await workspace.replaceTable(
98100
"errored",

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ pub fn error_message(p: &ErrorMessageProps) -> yew::Html {
3838
|| drop(sub)
3939
},
4040
);
41-
tracing::error!("Fucked");
4241
html! {
4342
<>
4443
<LocalStyle href={css!("render-warning")} />

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ impl PerspectiveViewerElement {
154154
/// ```
155155
pub fn load(&self, table: JsValue) -> ApiFuture<()> {
156156
tracing::info!("Loading Table");
157+
self.session.invalidate();
157158
let promise = table
158159
.clone()
159160
.dyn_into::<js_sys::Promise>()

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ impl Session {
580580
.borrow()
581581
.table
582582
.as_ref()
583-
.ok_or("`restore()` called before `load()`")?
583+
.ok_or("Trying to draw the viewer with no table attached")?
584584
.clone();
585585

586586
let valid_recs = table.validate_expressions(config.expressions).await?;
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
2+
// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃
3+
// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃
4+
// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃
5+
// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃
6+
// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
7+
// ┃ Copyright (c) 2017, the Perspective Authors. ┃
8+
// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃
9+
// ┃ This file is part of the Perspective library, distributed under the terms ┃
10+
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
11+
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
12+
13+
import { test, expect } from "@finos/perspective-test";
14+
15+
test.beforeEach(async ({ page }) => {
16+
await page.goto("/rust/perspective-viewer/test/html/blank.html");
17+
await page.waitForFunction(() => "WORKER" in window);
18+
});
19+
20+
test.describe("viewer.load() method", async () => {
21+
test("does load a resolved Table promise", async ({ page }) => {
22+
const viewer = page.locator("perspective-viewer");
23+
await viewer.evaluate(async (viewer) => {
24+
const goodTable = (await window.WORKER).table("a,b,c\n1,2,3");
25+
return viewer.load(goodTable);
26+
});
27+
await expect(viewer).toHaveText(/"a","b","c"/); // column titles
28+
});
29+
30+
test("is rejected by a rejected Table promise", async ({ page }) => {
31+
const viewer = page.locator("perspective-viewer");
32+
await expect(
33+
viewer.evaluate((viewer) => {
34+
const errorTable = Promise.reject(new Error("blimpy"));
35+
return viewer.load(errorTable);
36+
})
37+
).rejects.toThrow("blimpy");
38+
});
39+
40+
test("after a load error, same viewer can load a resolved Table promise", async ({
41+
page,
42+
}) => {
43+
const viewer = page.locator("perspective-viewer");
44+
const didError = await viewer.evaluate(async (viewer) => {
45+
const errorTable = Promise.reject(new Error("blimpy"));
46+
const worker = await window.WORKER;
47+
let didError = false;
48+
try {
49+
await viewer.load(errorTable);
50+
} catch (e) {
51+
if (e.message.includes("blimpy")) {
52+
didError = true;
53+
}
54+
}
55+
56+
const goodTable = worker.table("a,b,c\n1,2,3");
57+
await viewer.load(goodTable);
58+
return didError;
59+
});
60+
expect(didError).toBe(true);
61+
});
62+
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,11 @@ test.describe("Settings", () => {
111111

112112
consoleLogs.expectedLogs.push(
113113
"error",
114-
/Failed to apply config: Error: `restore\(\)` called before `load\(\)`.*/
114+
/Failed to apply config: Error: Trying to draw the viewer with no table attached/
115115
);
116116
consoleLogs.expectedLogs.push(
117117
"error",
118-
/Caught error: Error: `restore\(\)` called before `load\(\)`.*/
118+
/Caught error: Error: Trying to draw the viewer with no table attached/
119119
);
120120
});
121121
});

tools/perspective-scripts/test_js.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ function playwright(pkg, is_jlab) {
7676

7777
return sh`
7878
TZ=UTC
79-
npx playwright test
80-
--config=tools/perspective-test/playwright.config.ts
79+
npx playwright test
80+
--config=tools/perspective-test/playwright.config.ts
8181
${args}
8282
`.env(env);
8383
}
1.22 KB
Binary file not shown.

0 commit comments

Comments
 (0)