hub/sagas: fix error message not shown when starting repl

- Add check for repl start command success/fail.
- Show error message on fail.
- Rename start repl action.
- Add new did start/did fail to start repl actions.
- Change terminal saga to use did start repl action to focus terminal.
- Add more tests.

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Fixed
 - Fixed run button active while hub is disconnecting ([support#1021]).
+- Fixed error message not shown when starting REPL fails.
 
 [support#1021]: https://github.com/pybricks/support/issues/1021
 

src/hub/actions.ts

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020-2022 The Pybricks Authors
+// Copyright (c) 2020-2023 The Pybricks Authors
 
 import { createAction } from '../actions';
 import { FileFormat } from '../ble-pybricks-service/protocol';
@@ -43,7 +43,15 @@ export const stop = createAction(() => ({
     type: 'hub.action.stop',
 }));
 
-export const repl = createAction((useLegacyDownload: boolean) => ({
-    type: 'hub.action.repl',
+export const hubStartRepl = createAction((useLegacyDownload: boolean) => ({
+    type: 'hub.action.startRepl',
     useLegacyDownload,
 }));
+
+export const hubDidStartRepl = createAction(() => ({
+    type: 'hub.action.didStartRepl',
+}));
+
+export const hubDidFailToStartRepl = createAction(() => ({
+    type: 'hub.action.didFailToStartRepl',
+}));

src/hub/reducers.test.ts

@@ -14,6 +14,8 @@ import {
     didFinishDownload,
     didProgressDownload,
     didStartDownload,
+    hubDidFailToStartRepl,
+    hubStartRepl,
 } from './actions';
 import reducers, { HubRuntimeState } from './reducers';
 
@@ -170,4 +172,20 @@ describe('runtime', () => {
             ).runtime,
         ).toBe(HubRuntimeState.Idle);
     });
+
+    test('hubStartRepl', () => {
+        expect(
+            reducers({ runtime: HubRuntimeState.Running } as State, hubStartRepl(false))
+                .runtime,
+        ).toBe(HubRuntimeState.StartingRepl);
+    });
+
+    test('hubDidFailToStartRepl', () => {
+        expect(
+            reducers(
+                { runtime: HubRuntimeState.Running } as State,
+                hubDidFailToStartRepl(),
+            ).runtime,
+        ).toBe(HubRuntimeState.Unknown);
+    });
 });

src/hub/reducers.ts

@@ -26,36 +26,28 @@ import {
     didFinishDownload,
     didProgressDownload,
     didStartDownload,
+    hubDidFailToStartRepl,
+    hubStartRepl,
 } from './actions';
 
 /**
  * Describes the state of the MicroPython runtime on the hub.
  */
 export enum HubRuntimeState {
-    /**
-     * The hub is not connected.
-     */
+    /** The hub is not connected. */
     Disconnected = 'hub.runtime.disconnected',
-    /**
-     * The hub is connected but the state is not known yet.
-     */
+    /** The hub is connected but the state is not known yet. */
     Unknown = 'hub.runtime.unknown',
-    /**
-     * The runtime is idle waiting for command after soft reboot.
-     */
+    /** The runtime is idle waiting for command after soft reboot. */
     Idle = 'hub.runtime.idle',
-    /**
-     * A user program is being copied to the hub.
-     */
+    /** A user program is being copied to the hub. */
     Loading = 'hub.runtime.loading',
-    /**
-     * A user program has been copied to the hub.
-     */
+    /** A user program has been copied to the hub. */
     Loaded = 'hub.runtime.loaded',
-    /**
-     * A user program is running.
-     */
+    /** A user program is running. */
     Running = 'hub.runtime.running',
+    /** Busy starting the REPL. */
+    StartingRepl = 'hub.runtime.startingRepl',
 }
 
 const runtime: Reducer<HubRuntimeState> = (
@@ -116,6 +108,17 @@ const runtime: Reducer<HubRuntimeState> = (
         return HubRuntimeState.Idle;
     }
 
+    if (hubStartRepl.matches(action)) {
+        return HubRuntimeState.StartingRepl;
+    }
+
+    // NB: hubDidStartRepl will trigger user program running, so we don't
+    // change state for both to avoid race condition
+
+    if (hubDidFailToStartRepl.matches(action)) {
+        return HubRuntimeState.Unknown;
+    }
+
     return state;
 };
 

src/hub/sagas.test.ts

@@ -1,10 +1,13 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020-2022 The Pybricks Authors
+// Copyright (c) 2020-2023 The Pybricks Authors
 
 import { AsyncSaga } from '../../test';
+import { alertsShowAlert } from '../alerts/actions';
 import { didWrite, write } from '../ble-nordic-uart-service/actions';
 import {
+    didFailToSendCommand,
     didSendCommand,
+    sendStartReplCommand,
     sendStopUserProgramCommand,
 } from '../ble-pybricks-service/actions';
 import { FileFormat } from '../ble-pybricks-service/protocol';
@@ -17,7 +20,9 @@ import {
     didProgressDownload,
     didStartDownload,
     downloadAndRun,
-    repl,
+    hubDidFailToStartRepl,
+    hubDidStartRepl,
+    hubStartRepl,
     stop,
 } from './actions';
 import hub from './sagas';
@@ -83,15 +88,52 @@ describe('downloadAndRun', () => {
     // TODO: need to test error paths
 });
 
-test('repl', async () => {
-    const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
+describe('hubStartRepl', () => {
+    test('legacy UART download', async () => {
+        const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
 
-    saga.put(repl(true));
+        saga.put(hubStartRepl(true));
 
-    const action = await saga.take();
-    expect(action).toEqual(write(0, new Uint8Array([32, 32, 32, 32])));
+        await expect(saga.take()).resolves.toEqual(
+            write(0, new Uint8Array([32, 32, 32, 32])),
+        );
 
-    await saga.end();
+        await expect(saga.take()).resolves.toEqual(hubDidStartRepl());
+
+        await saga.end();
+    });
+
+    test('success', async () => {
+        const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
+
+        saga.put(hubStartRepl(false));
+
+        await expect(saga.take()).resolves.toEqual(sendStartReplCommand(0));
+
+        saga.put(didSendCommand(0));
+
+        await expect(saga.take()).resolves.toEqual(hubDidStartRepl());
+
+        await saga.end();
+    });
+
+    test('failure due to disconnect', async () => {
+        const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
+
+        saga.put(hubStartRepl(false));
+
+        await expect(saga.take()).resolves.toEqual(sendStartReplCommand(0));
+
+        saga.put(didFailToSendCommand(0, new DOMException('test', 'NetworkError')));
+
+        await expect(saga.take()).resolves.toEqual(
+            alertsShowAlert('ble', 'disconnected'),
+        );
+
+        await expect(saga.take()).resolves.toEqual(hubDidFailToStartRepl());
+
+        await saga.end();
+    });
 });
 
 test('stop', async () => {

src/hub/sagas.ts

@@ -46,7 +46,9 @@ import {
     didProgressDownload,
     didStartDownload,
     downloadAndRun,
-    repl,
+    hubDidFailToStartRepl,
+    hubDidStartRepl,
+    hubStartRepl,
     stop,
 } from './actions';
 
@@ -343,15 +345,46 @@ function* handleDownloadAndRun(action: ReturnType<typeof downloadAndRun>): Gener
 // SPACE, SPACE, SPACE, SPACE
 const legacyStartReplCommand = new Uint8Array([0x20, 0x20, 0x20, 0x20]);
 
-function* handleRepl(action: ReturnType<typeof repl>): Generator {
+function* handleHubStartRepl(action: ReturnType<typeof hubStartRepl>): Generator {
     const nextMessageId = yield* getContext<() => number>('nextMessageId');
 
     if (action.useLegacyDownload) {
         yield* put(write(nextMessageId(), legacyStartReplCommand));
+        yield* put(hubDidStartRepl());
         return;
     }
 
-    yield* put(sendStartReplCommand(nextMessageId()));
+    const id = nextMessageId();
+    yield* put(sendStartReplCommand(id));
+
+    const { didFailToSend } = yield* race({
+        didStart: take(didSendCommand.when((a) => a.id === id)),
+        didFailToSend: take(didFailToSendCommand.when((a) => a.id === id)),
+    });
+
+    if (didFailToSend) {
+        if (process.env.NODE_ENV !== 'test') {
+            console.error(didFailToSend.error);
+        }
+
+        if (
+            didFailToSend.error instanceof DOMException &&
+            didFailToSend.error.name === 'NetworkError'
+        ) {
+            yield* put(alertsShowAlert('ble', 'disconnected'));
+        } else {
+            yield* put(
+                alertsShowAlert('alerts', 'unexpectedError', {
+                    error: didFailToSend.error,
+                }),
+            );
+        }
+
+        yield* put(hubDidFailToStartRepl());
+        return;
+    }
+
+    yield* put(hubDidStartRepl());
 }
 
 function* handleStop(): Generator {
@@ -373,7 +406,7 @@ function* handleStop(): Generator {
 
 export default function* (): Generator {
     yield* takeEvery(downloadAndRun, handleDownloadAndRun);
-    yield* takeEvery(repl, handleRepl);
+    yield* takeEvery(hubStartRepl, handleHubStartRepl);
     yield* takeEvery(stop, handleStop);
     // calling stop right after connecting should get the hub into a known state
     yield* takeEvery(bleDidConnectPybricks, handleStop);

src/terminal/sagas.test.ts

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020-2022 The Pybricks Authors
+// Copyright (c) 2020-2023 The Pybricks Authors
 
 import PushStream from 'zen-push';
 import { AsyncSaga, delay } from '../../test';
@@ -9,7 +9,7 @@ import {
     didWrite,
     write,
 } from '../ble-nordic-uart-service/actions';
-import { checksum } from '../hub/actions';
+import { checksum, hubDidStartRepl } from '../hub/actions';
 import { HubRuntimeState } from '../hub/reducers';
 import { createCountFunc } from '../utils/iter';
 import { receiveData, sendData } from './actions';
@@ -179,3 +179,16 @@ describe('Terminal data source responds to receive data actions', () => {
         await saga.end();
     });
 });
+
+it('should focus terminal when repl is started', async () => {
+    const dispatchEventSpy = jest.spyOn(window, 'dispatchEvent');
+
+    const saga = new AsyncSaga(terminal);
+
+    saga.put(hubDidStartRepl());
+
+    // https://stackoverflow.com/a/64787979/1976323
+    expect(dispatchEventSpy.mock.calls[0][0].type).toBe('pb-terminal-focus');
+
+    await saga.end();
+});

src/terminal/sagas.ts

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020-2022 The Pybricks Authors
+// Copyright (c) 2020-2023 The Pybricks Authors
 
 import { AnyAction } from 'redux';
 import {
@@ -20,7 +20,7 @@ import {
     write,
 } from '../ble-nordic-uart-service/actions';
 import { nordicUartSafeTxCharLength } from '../ble-nordic-uart-service/protocol';
-import { checksum, repl } from '../hub/actions';
+import { checksum, hubDidStartRepl } from '../hub/actions';
 import { HubRuntimeState } from '../hub/reducers';
 import { RootState } from '../reducers';
 import { defined } from '../utils';
@@ -109,7 +109,7 @@ function* sendTerminalData(action: ReturnType<typeof sendData>): Generator {
     dataSource.next(action.value);
 }
 
-function handleRepl(): void {
+function handleHubDidStartRepl(): void {
     dispatchEvent(new CustomEvent('pb-terminal-focus'));
 }
 
@@ -117,5 +117,5 @@ export default function* (): Generator {
     yield* takeEvery(didNotify, receiveUartData);
     yield* fork(receiveTerminalData);
     yield* takeEvery(sendData, sendTerminalData);
-    yield* takeEvery(repl, handleRepl);
+    yield* takeEvery(hubDidStartRepl, handleHubDidStartRepl);
 }

src/toolbar/buttons/repl/ReplButton.test.tsx

@@ -4,7 +4,7 @@
 import { act, cleanup } from '@testing-library/react';
 import React from 'react';
 import { testRender } from '../../../../test';
-import { repl } from '../../../hub/actions';
+import { hubStartRepl } from '../../../hub/actions';
 import { HubRuntimeState } from '../../../hub/reducers';
 import ReplButton from './ReplButton';
 
@@ -19,5 +19,5 @@ it('should dispatch action when clicked', async () => {
 
     await act(() => user.click(button.getByRole('button', { name: 'REPL' })));
 
-    expect(dispatch).toHaveBeenCalledWith(repl(false));
+    expect(dispatch).toHaveBeenCalledWith(hubStartRepl(false));
 });

src/toolbar/buttons/repl/ReplButton.tsx

@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020-2022 The Pybricks Authors
+// Copyright (c) 2020-2023 The Pybricks Authors
 
 import React, { useCallback } from 'react';
 import { useDispatch } from 'react-redux';
-import { repl } from '../../../hub/actions';
+import { hubStartRepl } from '../../../hub/actions';
 import { HubRuntimeState } from '../../../hub/reducers';
 import { useSelector } from '../../../reducers';
 import ActionButton, { ActionButtonProps } from '../../ActionButton';
@@ -18,7 +18,7 @@ const ReplButton: React.VoidFunctionComponent<ReplButtonProps> = ({ id }) => {
     const dispatch = useDispatch();
 
     const action = useCallback(
-        () => dispatch(repl(useLegacyDownload)),
+        () => dispatch(hubStartRepl(useLegacyDownload)),
         [dispatch, useLegacyDownload],
     );
 
@@ -29,6 +29,7 @@ const ReplButton: React.VoidFunctionComponent<ReplButtonProps> = ({ id }) => {
             tooltip={i18n.translate('tooltip')}
             icon={icon}
             enabled={hasRepl && runtime === HubRuntimeState.Idle}
+            showProgress={runtime === HubRuntimeState.StartingRepl}
             onAction={action}
         />
     );