Fix cancellable methods in perspective-viewer

cpp/perspective/src/cpp/server.cpp

@@ -423,6 +423,10 @@ ServerResources::get_table_for_view(const t_id& view_id) {
 ServerResources::t_id
 ServerResources::get_table_id_for_view(const t_id& view_id) {
     PSP_READ_LOCK(m_write_lock);
+    if (!m_view_to_table.contains(view_id)) {
+        throw PerspectiveViewNotFoundException();
+    }
+
     return m_view_to_table.at(view_id);
 }
 
@@ -440,6 +444,10 @@ ServerResources::get_view_ids(const t_id& table_id) {
 std::shared_ptr<ErasedView>
 ServerResources::get_view(const t_id& id) {
     PSP_READ_LOCK(m_write_lock);
+    if (!m_views.contains(id)) {
+        throw PerspectiveViewNotFoundException();
+    }
+
     return m_views.at(id);
 }
 
@@ -680,6 +688,13 @@ ProtoServer::handle_request(
         auto* err = resp.mutable_server_error()->mutable_message();
         *err = std::string(e.what());
         responses.emplace_back(std::move(resp));
+    } catch (const PerspectiveViewNotFoundException& e) {
+        proto::Response resp;
+        auto* err = resp.mutable_server_error();
+        err->set_status_code(proto::StatusCode::VIEW_NOT_FOUND);
+        auto* msg = err->mutable_message();
+        *msg = std::string(e.what());
+        responses.emplace_back(std::move(resp));
     } catch (const std::exception& e) {
         proto::Response resp;
         auto* err = resp.mutable_server_error()->mutable_message();

cpp/perspective/src/include/perspective/exception.h

@@ -28,4 +28,16 @@ class PERSPECTIVE_EXPORT PerspectiveException : public std::exception {
     std::string message;
 };
 
+class PERSPECTIVE_EXPORT PerspectiveViewNotFoundException
+    : public std::exception {
+public:
+    explicit PerspectiveViewNotFoundException() {}
+
+    [[nodiscard]]
+    const char*
+    what() const noexcept override {
+        return "View not found";
+    }
+};
+
 } // namespace perspective

cpp/protos/perspective.proto

@@ -22,9 +22,15 @@ option optimize_for = LITE_RUNTIME;
 //
 // Common
 
+enum StatusCode {
+    SERVER_ERROR = 0;
+    VIEW_NOT_FOUND = 1;
+}
+
 // Recoverable, user-readable error reporting from the engine.
 message ServerError {
     string message = 1;
+    StatusCode status_code = 2;
 }
 
 message Schema {

rust/perspective-client/src/rust/utils/mod.rs

@@ -22,6 +22,9 @@ use crate::proto;
 
 #[derive(Error, Debug)]
 pub enum ClientError {
+    #[error("View not found")]
+    ViewNotFound,
+
     #[error("Abort(): {0}")]
     Internal(String),
 
@@ -64,7 +67,10 @@ pub type ClientResult<T> = Result<T, ClientError>;
 impl From<proto::response::ClientResp> for ClientError {
     fn from(value: proto::response::ClientResp) -> Self {
         match value {
-            proto::response::ClientResp::ServerError(x) => ClientError::Internal(x.message),
+            proto::response::ClientResp::ServerError(x) => match x.status_code() {
+                proto::StatusCode::ServerError => ClientError::Internal(x.message),
+                proto::StatusCode::ViewNotFound => ClientError::ViewNotFound,
+            },
             x => ClientError::ResponseFailed(Box::new(x)),
         }
     }

rust/perspective-js/src/rust/table.rs

@@ -186,7 +186,7 @@ impl Table {
 
     #[doc = inherit_docs!("table/delete.md")]
     #[wasm_bindgen]
-    pub async fn delete(self) -> ApiResult<()> {
+    pub async fn delete(&self) -> ApiResult<()> {
         self.0.delete().await?;
         Ok(())
     }

rust/perspective-js/src/rust/utils/errors.rs

@@ -12,6 +12,7 @@
 
 use std::fmt::Display;
 
+use perspective_client::ClientError;
 use wasm_bindgen::prelude::*;
 
 /// A bespoke error class for chaining a litany of various error types with the
@@ -78,7 +79,6 @@ define_api_error!(
     serde_json::Error,
     rmp_serde::encode::Error,
     rmp_serde::decode::Error,
-    perspective_client::ClientError,
     &str,
     String,
     futures::channel::oneshot::Canceled,
@@ -87,6 +87,16 @@ define_api_error!(
     prost::DecodeError
 );
 
+#[wasm_bindgen(inline_js = r#"
+export class PerspectiveViewNotFoundError extends Error {}
+"#)]
+extern "C" {
+    pub type PerspectiveViewNotFoundError;
+
+    #[wasm_bindgen(constructor)]
+    fn new() -> PerspectiveViewNotFoundError;
+}
+
 /// Explicit conversion methods for `ApiResult<T>`, for situations where
 /// error-casting through the `?` operator is insufficient.
 pub trait ToApiError<T> {
@@ -105,3 +115,12 @@ impl ToApiError<JsValue> for Result<(), ApiResult<JsValue>> {
         self.map_or_else(|x| x, |()| Ok(JsValue::UNDEFINED))
     }
 }
+
+impl From<perspective_client::ClientError> for ApiError {
+    fn from(value: ClientError) -> Self {
+        match value {
+            ClientError::ViewNotFound => ApiError(PerspectiveViewNotFoundError::new().into()),
+            err => ApiError(JsError::new(format!("{}", err).as_str()).into()),
+        }
+    }
+}

rust/perspective-js/src/rust/utils/futures.rs

@@ -84,9 +84,11 @@ where
 {
     fn from(fut: ApiFuture<T>) -> Self {
         future_to_promise(async move {
-            let x = Ok(fut.0.await?);
-            let y = Ok(x.into_js_result()?);
-            Ok(y.ignore_view_delete()?)
+            fut.0
+                .await
+                .map_err(|x| x.into())
+                .into_js_result()
+                .ignore_view_delete()
         })
     }
 }
@@ -146,7 +148,7 @@ where
 static CANCELLED_MSG: &str = "View method cancelled";
 
 #[extend::ext]
-pub impl Result<JsValue, ApiError> {
+pub impl Result<JsValue, JsValue> {
     /// Wraps an error `JsValue` return from a caught JavaScript exception,
     /// checking for the explicit error type indicating that a
     /// `JsPerspectiveView` call has been cancelled due to it already being
@@ -160,35 +162,22 @@ pub impl Result<JsValue, ApiError> {
     /// silently be replaced with `Ok`.  The message itself is returned in this
     /// case (instead of whatever the `async` returns), which is helpful for
     /// detecting this condition when debugging.
+    fn ignore_view_delete(self) -> Result<JsValue, JsValue> {
+        self.or_else(|x| match x.dyn_ref::<PerspectiveViewNotFoundError>() {
+            Some(_) => Ok(js_intern::js_intern!(CANCELLED_MSG).clone()),
+            None => Err(x),
+        })
+    }
+}
+
+#[extend::ext]
+pub impl Result<JsValue, ApiError> {
     fn ignore_view_delete(self) -> Result<JsValue, ApiError> {
         self.or_else(|x| {
             let f: JsValue = x.clone().into();
-            match f.dyn_ref::<js_sys::Error>() {
-                Some(err) => {
-                    if err.message() != CANCELLED_MSG {
-                        Err(x)
-                    } else {
-                        Ok(js_intern::js_intern!(CANCELLED_MSG).clone())
-                    }
-                },
-                _ => match f.as_string() {
-                    Some(x) if x == CANCELLED_MSG => {
-                        Ok(js_intern::js_intern!(CANCELLED_MSG).clone())
-                    },
-                    Some(_) => Err(x),
-                    _ => {
-                        if js_sys::Reflect::get(&f, js_intern::js_intern!("message"))
-                            .unwrap()
-                            .as_string()
-                            .unwrap_or_else(|| "".to_owned())
-                            == CANCELLED_MSG
-                        {
-                            Ok(js_intern::js_intern!(CANCELLED_MSG).clone())
-                        } else {
-                            Err(x)
-                        }
-                    },
-                },
+            match f.dyn_ref::<PerspectiveViewNotFoundError>() {
+                Some(_) => Ok(js_intern::js_intern!(CANCELLED_MSG).clone()),
+                None => Err(x),
             }
         })
     }

rust/perspective-js/src/rust/view.rs

@@ -70,7 +70,7 @@ impl View {
 
     #[doc = inherit_docs!("view/delete.md")]
     #[wasm_bindgen]
-    pub async fn delete(self) -> ApiResult<()> {
+    pub async fn delete(&self) -> ApiResult<()> {
         self.0.delete().await?;
         Ok(())
     }

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

@@ -65,13 +65,18 @@ impl PerspectiveDebugPluginElement {
         JsValue::UNDEFINED
     }
 
-    pub fn update(&self, view: perspective_js::View) -> ApiFuture<()> {
+    pub fn update(&self, view: &perspective_js::View) -> ApiFuture<()> {
         self.draw(view)
     }
 
-    pub fn draw(&self, view: perspective_js::View) -> ApiFuture<()> {
+    /// # Notes
+    ///
+    /// When you pass a `wasm_bindgen` wrapped type _into_ Rust, it acts like a
+    /// move. Ergo, if you replace the `&` in the `view` argument, the JS copy
+    /// of the `View` will be invalid
+    pub fn draw(&self, view: &perspective_js::View) -> ApiFuture<()> {
         let css = "margin:0;overflow:scroll;position:absolute;width:100%;height:100%";
-        clone!(self.elem);
+        clone!(self.elem, view);
         ApiFuture::new(async move {
             let csv = view.to_csv(None).await?;
             elem.style().set_property("background-color", "#fff")?;

rust/perspective-viewer/src/ts/plugin.ts

@@ -122,7 +122,7 @@ export interface IPerspectiveViewerPlugin {
      * }
      * ```
      */
-    // draw(view: perspective.View): Promise<void>;
+    draw(view: View): Promise<void>;
 
     /**
      * Draw under the assumption that the `ViewConfig` has not changed since
@@ -136,7 +136,7 @@ export interface IPerspectiveViewerPlugin {
      * }
      * ```
      */
-    // update(view: perspective.View): Promise<void>;
+    update(view: View): Promise<void>;
 
     /**
      * Clear this plugin, though it is up to the discretion of the plugin

rust/perspective-viewer/test/html/plugin-resize.html

@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script type="module" src="/node_modules/@finos/perspective-viewer/dist/cdn/perspective-viewer.js"></script>
+        <script type="module">
+            await customElements.whenDefined("perspective-viewer-plugin");
+            const BASE = customElements.get("perspective-viewer-plugin");
+
+            class TestPluginImplementsResize extends BASE {
+                get name() {
+                    return "Resizing Plugin";
+                }
+
+                async draw(view) {
+                    this._view = view;
+                    console.log(await this._view.num_rows());
+                    return await super.draw(view);
+                }
+
+                async update(view) {
+                    this._view = view;
+                    console.log(await this._view.num_rows());
+                    return await super.update(view);
+                }
+
+                async resize(view) {
+                    console.log(await this._view.num_rows());
+                    await this._view.to_csv();
+                }
+            }
+
+            customElements.define("resize-plugin", TestPluginImplementsResize);
+            await customElements.whenDefined("perspective-viewer").then((viewer) => {
+                viewer.registerPlugin("resize-plugin");
+            });
+        </script>
+
+        <script type="module" src="/node_modules/@finos/perspective-test/load-viewer-csv.js"></script>
+        <link rel="stylesheet" href="../css/demo.css" />
+        <link rel="stylesheet" href="/node_modules/@finos/perspective-viewer/dist/css/pro.css" />
+    </head>
+    <body>
+        <perspective-viewer></perspective-viewer>
+    </body>
+</html>

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

@@ -0,0 +1,57 @@
+// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
+// ┃ ██████ ██████ ██████       █      █      █      █      █ █▄  ▀███ █       ┃
+// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█  ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄  ▀█ █ ▀▀▀▀▀ ┃
+// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄   █ ▄▄▄▄▄ ┃
+// ┃ █      ██████ █  ▀█▄       █ ██████      █      ███▌▐███ ███████▄ █       ┃
+// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
+// ┃ 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 } from "@finos/perspective-test";
+import { compareContentsToSnapshot } from "@finos/perspective-test";
+
+async function get_contents(page) {
+    return await page.evaluate(async () => {
+        const viewer = document
+            .querySelector("perspective-viewer")
+            .shadowRoot.querySelector("#app_panel");
+        return viewer ? viewer.innerHTML : "MISSING";
+    });
+}
+
+test.beforeEach(async ({ page }) => {
+    await page.goto("/rust/perspective-viewer/test/html/plugin-resize.html");
+    await page.evaluate(async () => {
+        while (!window["__TEST_PERSPECTIVE_READY__"]) {
+            await new Promise((x) => setTimeout(x, 10));
+        }
+    });
+});
+
+test.describe("Cancellable methods", () => {
+    test("Cancellable view methods do not error", async ({ page }) => {
+        await page.evaluate(async () => {
+            const viewer = document.querySelector("perspective-viewer");
+            await viewer.restore({
+                group_by: ["State"],
+                columns: ["Sales"],
+                settings: true,
+                filter: [
+                    ["State", "not in", ["California", "Texas", "New York"]],
+                ],
+            });
+
+            const view = await viewer.getView();
+            await view.delete();
+            await viewer.resize(true);
+        });
+
+        const contents = await get_contents(page);
+        await compareContentsToSnapshot(contents, [
+            "regressions-not_in-filter-works-correctly.txt",
+        ]);
+    });
+});