Fix rejected promise behavior in interpreter and compiler

Before this command:
- Interpreter would popStack() and continue. Inside a loop this was like JS continue, outside it was like return
- Compiler would return literal undefined and keep going.

After this commit:
Both return the error as a string and then continue as normal.
This has several benefits:
- Users can actually see the error messages
- Logical execution flow
- Consistent behavior
- Blocks aren't even supposed to return rejected promises anyways
- No vanilla blocks return rejected promises so no compatibility concerns

Supersedes #207

src/compiler/jsexecute.js

@@ -119,8 +119,9 @@ const waitPromise = function*(promise) {
             returnValue = value;
             thread.status = 0; // STATUS_RUNNING
         }, error => {
-            thread.status = 0; // STATUS_RUNNING
             globalState.log.warn('Promise rejected in compiled script:', error);
+            returnValue = '' + error;
+            thread.status = 0; // STATUS_RUNNING
         });
 
     yield;

src/engine/execute.js

@@ -112,45 +112,47 @@ const handleReport = function (resolvedValue, sequencer, thread, blockCached, la
     }
 };
 
+const handlePromiseResolution = (resolvedValue, sequencer, thread, blockCached, lastOperation) => {
+    handleReport(resolvedValue, sequencer, thread, blockCached, lastOperation);
+    // If it's a command block or a top level reporter in a stackClick.
+    // TW: Don't mangle the stack when we just finished executing a hat block.
+    // Hat block is always the top and first block of the script. There are no loops to find.
+    if (lastOperation && (!blockCached._isHat || thread.stackClick)) {
+        let stackFrame;
+        let nextBlockId;
+        do {
+            // In the case that the promise is the last block in the current thread stack
+            // We need to pop out repeatedly until we find the next block.
+            const popped = thread.popStack();
+            if (popped === null) {
+                return;
+            }
+            nextBlockId = thread.target.blocks.getNextBlock(popped);
+            if (nextBlockId !== null) {
+                // A next block exists so break out this loop
+                break;
+            }
+            // Investigate the next block and if not in a loop,
+            // then repeat and pop the next item off the stack frame
+            stackFrame = thread.peekStackFrame();
+        } while (stackFrame !== null && !stackFrame.isLoop);
+
+        thread.pushStack(nextBlockId);
+    }
+};
+
 const handlePromise = (primitiveReportedValue, sequencer, thread, blockCached, lastOperation) => {
     if (thread.status === Thread.STATUS_RUNNING) {
         // Primitive returned a promise; automatically yield thread.
         thread.status = Thread.STATUS_PROMISE_WAIT;
     }
     // Promise handlers
     primitiveReportedValue.then(resolvedValue => {
-        handleReport(resolvedValue, sequencer, thread, blockCached, lastOperation);
-        // If it's a command block or a top level reporter in a stackClick.
-        // TW: Don't mangle the stack when we just finished executing a hat block.
-        // Hat block is always the top and first block of the script. There are no loops to find.
-        if (lastOperation && (!blockCached._isHat || thread.stackClick)) {
-            let stackFrame;
-            let nextBlockId;
-            do {
-                // In the case that the promise is the last block in the current thread stack
-                // We need to pop out repeatedly until we find the next block.
-                const popped = thread.popStack();
-                if (popped === null) {
-                    return;
-                }
-                nextBlockId = thread.target.blocks.getNextBlock(popped);
-                if (nextBlockId !== null) {
-                    // A next block exists so break out this loop
-                    break;
-                }
-                // Investigate the next block and if not in a loop,
-                // then repeat and pop the next item off the stack frame
-                stackFrame = thread.peekStackFrame();
-            } while (stackFrame !== null && !stackFrame.isLoop);
-
-            thread.pushStack(nextBlockId);
-        }
+        handlePromiseResolution(resolvedValue, sequencer, thread, blockCached, lastOperation);
     }, rejectionReason => {
         // Promise rejected: the primitive had some error.
-        // Log it and proceed.
         log.warn('Primitive rejected promise: ', rejectionReason);
-        thread.status = Thread.STATUS_RUNNING;
-        thread.popStack();
+        handlePromiseResolution(`${rejectionReason}`, sequencer, thread, blockCached, lastOperation);
     });
 };
 

test/integration/tw_rejected_promise.js

@@ -0,0 +1,102 @@
+const {test} = require('tap');
+const fs = require('fs');
+const path = require('path');
+const VM = require('../../src/virtual-machine');
+const Scratch = require('../../src/extension-support/tw-extension-api-common');
+
+const commandFixture = fs.readFileSync(path.join(__dirname, '../fixtures/tw-rejected-promise-command.sb3'));
+const reporterFixture = fs.readFileSync(path.join(__dirname, '../fixtures/tw-rejected-promise-reporter.sb3'));
+
+class TestExtension {
+    getInfo () {
+        return {
+            id: 'test123',
+            name: 'test123',
+            blocks: [
+                {
+                    blockType: Scratch.BlockType.COMMAND,
+                    opcode: 'command',
+                    text: 'return rejected promise'
+                },
+                {
+                    blockType: Scratch.BlockType.REPORTER,
+                    opcode: 'reporter',
+                    text: 'return rejected promise'
+                }
+            ]
+        };
+    }
+    command () {
+        return Promise.reject(new Error('Test error 1'));
+    }
+    reporter () {
+        return Promise.reject(new Error('Test error 2'));
+    }
+}
+
+for (const enableCompiler of [true, false]) {
+    test(`COMMAND returns rejected promise - ${enableCompiler ? 'compiler' : 'interpreter'}`, t => {
+        const vm = new VM();
+        vm.extensionManager.addBuiltinExtension('test123', TestExtension);
+
+        vm.setCompilerOptions({
+            enabled: enableCompiler
+        });
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        vm.loadProject(commandFixture).then(async () => {
+            vm.greenFlag();
+
+            for (let i = 0; i < 12; i++) {
+                vm.runtime._step();
+
+                // wait for promise rejection to be handled
+                await Promise.resolve();
+            }
+
+            const stage = vm.runtime.getTargetForStage();
+            t.equal(stage.lookupVariableByNameAndType('before', '').value, 10);
+            t.equal(stage.lookupVariableByNameAndType('after', '').value, 10);
+            t.end();
+        });
+    });
+
+    test(`REPORTER returns rejected promise - ${enableCompiler ? 'compiler' : 'interpreter'}`, t => {
+        const vm = new VM();
+        vm.extensionManager.addBuiltinExtension('test123', TestExtension);
+
+        vm.setCompilerOptions({
+            enabled: enableCompiler
+        });
+        t.equal(vm.runtime.compilerOptions.enabled, enableCompiler);
+
+        vm.loadProject(reporterFixture).then(async () => {
+            vm.greenFlag();
+
+            for (let i = 0; i < 12; i++) {
+                vm.runtime._step();
+
+                // wait for promise rejection to be handled
+                await Promise.resolve();
+            }
+
+            const stage = vm.runtime.getTargetForStage();
+            t.equal(stage.lookupVariableByNameAndType('before', '').value, 10);
+            t.equal(stage.lookupVariableByNameAndType('after', '').value, 10);
+            t.same(stage.lookupVariableByNameAndType('values', 'list').value, [
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2',
+                'Error: Test error 2'
+            ]);
+            t.end();
+        });
+    });
+}