Repository: sjbarag/brs
Pull request #547 (2020-09-24 18:36:30)

fix(error-handling): Use addError instead of throw
# Change Summary

This calls `interpreter.addError` instead of `throw new BrsError` in several spots so that we get these runtime errors printed out to the console when they occur. Also, it removes a `finally` block so that we are explicitly swallowing errors, rather than doing it implicitly.


Patch:
diff --git a/src/interpreter/index.ts b/src/interpreter/index.ts
index 012b972..0cd123c 100644
--- a/src/interpreter/index.ts
+++ b/src/interpreter/index.ts
@@ -241,12 +241,13 @@ export class Interpreter implements Expr.Visitor<BrsType>, Stmt.Visitor<BrsType>
         } catch (err) {
             if (err instanceof Stmt.ReturnValue) {
                 results = [err.value || BrsInvalid.Instance];
-            } else {
+            } else if (!(err instanceof BrsError)) {
+                // Swallow BrsErrors, because they should have been exposed to the user downstream.
                 throw err;
             }
-        } finally {
-            return results;
         }
+
+        return results;
     }
 
     getCallableFunction(functionName: string): Callable | undefined {
@@ -1210,28 +1211,33 @@ export class Interpreter implements Expr.Visitor<BrsType>, Stmt.Visitor<BrsType>
     visitIndexedGet(expression: Expr.IndexedGet): BrsType {
         let source = this.evaluate(expression.obj);
         if (!isIterable(source)) {
-            throw new TypeMismatch({
-                message: "Attempting to retrieve property from non-iterable value",
-                left: {
-                    type: source,
-                    location: expression.location,
-                },
-            });
+            this.addError(
+                new TypeMismatch({
+                    message: "Attempting to retrieve property from non-iterable value",
+                    left: {
+                        type: source,
+                        location: expression.location,
+                    },
+                })
+            );
         }
 
         let index = this.evaluate(expression.index);
         if (!isBrsNumber(index) && !isBrsString(index)) {
-            throw new TypeMismatch({
-                message: "Attempting to retrieve property from iterable with illegal index type",
-                left: {
-                    type: source,
-                    location: expression.obj.location,
-                },
-                right: {
-                    type: index,
-                    location: expression.index.location,
-                },
-            });
+            this.addError(
+                new TypeMismatch({
+                    message:
+                        "Attempting to retrieve property from iterable with illegal index type",
+                    left: {
+                        type: source,
+                        location: expression.obj.location,
+                    },
+                    right: {
+                        type: index,
+                        location: expression.index.location,
+                    },
+                })
+            );
         }
 
         try {


Test patch:
diff --git a/test/e2e/Errors.test.js b/test/e2e/Errors.test.js
new file mode 100644
index 0000000..0a1ff63
--- /dev/null
+++ b/test/e2e/Errors.test.js
@@ -0,0 +1,48 @@
+const { execute } = require("../../lib");
+const { createMockStreams, resourceFile, allArgs } = require("./E2ETests");
+
+describe("Runtime errors", () => {
+    let outputStreams;
+
+    beforeAll(() => {
+        outputStreams = createMockStreams();
+        outputStreams.root = __dirname + "/resources";
+    });
+
+    afterEach(() => {
+        jest.resetAllMocks();
+    });
+
+    afterAll(() => {
+        jest.restoreAllMocks();
+    });
+
+    test("components/errors/dotted-get.brs", async () => {
+        await execute([resourceFile("components", "errors", "dotted-get.brs")], outputStreams);
+
+        let errOutput = allArgs(outputStreams.stderr.write).filter((arg) => arg !== "\n");
+        expect(
+            errOutput[0].includes("Attempting to retrieve property from non-iterable value")
+        ).toBeTruthy();
+    });
+
+    test("components/errors/indexed-get.brs", async () => {
+        await execute([resourceFile("components", "errors", "indexed-get.brs")], outputStreams);
+
+        let errOutput = allArgs(outputStreams.stderr.write).filter((arg) => arg !== "\n");
+        expect(
+            errOutput[0].includes("Attempting to retrieve property from non-iterable value")
+        ).toBeTruthy();
+    });
+
+    test("components/errors/illegal-index.brs", async () => {
+        await execute([resourceFile("components", "errors", "illegal-index.brs")], outputStreams);
+
+        let errOutput = allArgs(outputStreams.stderr.write).filter((arg) => arg !== "\n");
+        expect(
+            errOutput[0].includes(
+                "Attempting to retrieve property from iterable with illegal index type"
+            )
+        ).toBeTruthy();
+    });
+});
diff --git a/test/e2e/resources/components/errors/dotted-get.brs b/test/e2e/resources/components/errors/dotted-get.brs
new file mode 100644
index 0000000..a877f4f
--- /dev/null
+++ b/test/e2e/resources/components/errors/dotted-get.brs
@@ -0,0 +1,3 @@
+function main()
+    print foo.bar
+end function
diff --git a/test/e2e/resources/components/errors/illegal-index.brs b/test/e2e/resources/components/errors/illegal-index.brs
new file mode 100644
index 0000000..a78bdf5
--- /dev/null
+++ b/test/e2e/resources/components/errors/illegal-index.brs
@@ -0,0 +1,5 @@
+function main()
+    foo = ["bar"]
+    baz = {}
+    print foo[baz]
+end function
diff --git a/test/e2e/resources/components/errors/indexed-get.brs b/test/e2e/resources/components/errors/indexed-get.brs
new file mode 100644
index 0000000..a877f4f
--- /dev/null
+++ b/test/e2e/resources/components/errors/indexed-get.brs
@@ -0,0 +1,3 @@
+function main()
+    print foo.bar
+end function