Merge pull request #822 from finos/fix-callbacks

Properly remove `on_delete` and `on_update` callbacks that fail.

.gitignore

@@ -155,5 +155,6 @@ python/cpp
 python/perspective/perspective_python.egg-info
 python/perspective/perspective/tests/table/psp_test
 python/perspective/perspective/node/assets/*
+/__pycache__
 docs/static/js/logo.js
 docs/static/js/logo.js.map

packages/perspective/src/js/api/server.js

@@ -166,8 +166,9 @@ export class Server {
 
                         this.post(result);
                     } catch (e) {
-                        console.error("Removing callback after failed on_update() (presumably due to closed connection)");
-                        obj["remove_update"](callback);
+                        console.error(`Removing failed callback to \`${msg.method}()\` (presumably due to failed connection)`);
+                        const remove_method = msg.method.substring(3);
+                        obj[`remove_${remove_method}`](callback);
                     }
                 };
                 if (msg.callback_id) {

packages/perspective/test/js/delete.js

@@ -76,5 +76,31 @@ module.exports = perspective => {
 
             await table.delete();
         });
+
+        it("properly removes a failed delete callback on a table", async function(done) {
+            const table = perspective.table([{x: 1}]);
+
+            // when a callback throws, it should delete that callback
+            table.on_delete(() => {
+                throw new Error("something went wrong!");
+            });
+
+            table.delete();
+            done();
+        });
+
+        it("properly removes a failed delete callback on a view", async function(done) {
+            const table = perspective.table([{x: 1}]);
+            const view = table.view();
+
+            // when a callback throws, it should delete that callback
+            view.on_delete(() => {
+                throw new Error("something went wrong!");
+            });
+
+            view.delete();
+            table.delete();
+            done();
+        });
     });
 };

packages/perspective/test/js/updates.js

@@ -395,6 +395,39 @@ module.exports = perspective => {
             );
             table1.update(data);
         });
+
+        it("properly removes a failed update callback on a table", async function(done) {
+            const table = perspective.table({x: "integer"});
+            const view = table.view();
+            let size = await table.size();
+            let counter = 0;
+
+            // when a callback throws, it should delete that callback
+            view.on_update(() => {
+                counter++;
+                throw new Error("something went wrong!");
+            });
+
+            view.on_update(async () => {
+                // failed callback gets removed; non-failing callback gets called
+                let sz = await table.size();
+                expect(counter).toEqual(1);
+                expect(sz).toEqual(size++);
+            });
+
+            table.update([{x: 1}]);
+            table.update([{x: 2}]);
+            table.update([{x: 3}]);
+
+            const view2 = table.view(); // create a new view to make sure we process every update transacation.
+            const final_size = await table.size();
+            expect(final_size).toEqual(3);
+
+            view2.delete();
+            view.delete();
+            table.delete();
+            done();
+        });
     });
 
     describe("Limit", function() {