From 4707b77135698d7e7c549db3148e7943a4a9b1f4 Mon Sep 17 00:00:00 2001
From: David Lechner <david@pybricks.com>
Date: Fri, 22 Jan 2021 11:49:51 -0600
Subject: [PATCH] proper error handling for compiler error

---
 src/sagas/flash-firmware.test.ts | 57 +++++++++++++++++++++++++++++++-
 src/sagas/flash-firmware.ts      |  8 +++--
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/src/sagas/flash-firmware.test.ts b/src/sagas/flash-firmware.test.ts
index 5fd20359..a18de881 100644
--- a/src/sagas/flash-firmware.test.ts
+++ b/src/sagas/flash-firmware.test.ts
@@ -34,7 +34,7 @@ import {
     programResponse,
     rebootRequest,
 } from '../actions/lwp3-bootloader';
-import { didCompile } from '../actions/mpy';
+import { didCompile, didFailToCompile } from '../actions/mpy';
 import { HubType, Result } from '../protocols/lwp3-bootloader';
 import { createCountFunc } from '../utils/iter';
 import flashFirmware from './flash-firmware';
@@ -411,6 +411,61 @@ describe('flashFirmware', () => {
 
             await saga.end();
         });
+
+        test('compile error', async () => {
+            const metadata: FirmwareMetadata = {
+                'metadata-version': '1.0.0',
+                'device-id': HubType.MoveHub,
+                'checksum-type': 'sum',
+                'firmware-version': '1.2.3',
+                'max-firmware-size': 1024,
+                'mpy-abi-version': 5,
+                'mpy-cross-options': ['-mno-unicode'],
+                'user-mpy-offset': 100,
+            };
+
+            const zip = new JSZip();
+            zip.file('firmware-base.bin', new Uint8Array(64));
+            zip.file('firmware.metadata.json', JSON.stringify(metadata));
+            zip.file('main.py', 'print("test")');
+            zip.file('ReadMe_OSS.txt', 'test');
+
+            const saga = new AsyncSaga(flashFirmware, {
+                nextMessageId: createCountFunc(),
+            });
+
+            saga.setState({ settings: { flashCurrentProgram: false } });
+
+            // saga is triggered by this action
+
+            saga.put(
+                flashFirmwareAction(await zip.generateAsync({ type: 'arraybuffer' })),
+            );
+
+            // the first step is to compile main.py to .mpy
+
+            let action = await saga.take();
+            expect(action).toMatchInlineSnapshot(`
+                Object {
+                  "options": Array [
+                    "-mno-unicode",
+                  ],
+                  "script": "print(\\"test\\")",
+                  "type": "mpy.action.compile",
+                }
+            `);
+
+            saga.put(didFailToCompile(['test']));
+
+            // compiler error should trigger firmware flash failure
+
+            action = await saga.take();
+            expect(action).toEqual(
+                didFailToStart(FailToStartReasonType.FailedToCompile),
+            );
+
+            await saga.end();
+        });
     });
 
     test('user supplied main.py', async () => {
diff --git a/src/sagas/flash-firmware.ts b/src/sagas/flash-firmware.ts
index 6e368b8b..2369e6d6 100644
--- a/src/sagas/flash-firmware.ts
+++ b/src/sagas/flash-firmware.ts
@@ -109,7 +109,10 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator<number> {
 }
 
 /**
- * Loads Pybricks firmware from a .zip file
+ * Loads Pybricks firmware from a .zip file.
+ *
+ * This can raise didFailToStart() actions, so don't call this after didStart().
+ *
  * @param data The zip file raw data
  * @param program User program or `undefined` to use main.py from firmware.zip
  */
@@ -157,7 +160,8 @@ function* loadFirmware(
     });
 
     if (mpyFail) {
-        throw Error(mpyFail.err.join('\n'));
+        yield* put(didFailToStart(FailToStartReasonType.FailedToCompile));
+        yield* cancel();
     }
 
     defined(mpy);