hub/sagas: show error if stopping failed

- Finish TODO to show error if stopping fails.
- Make button busy in case waiting for failure takes a long time.

src/hub/actions.ts

@@ -39,19 +39,33 @@ export const didFailToFinishDownload = createAction(() => ({
     type: 'hub.action.didFailToFinishDownload',
 }));
 
-export const stop = createAction(() => ({
-    type: 'hub.action.stop',
+/** Request to send the stop user program command to the hub. */
+export const hubStopUserProgram = createAction(() => ({
+    type: 'hub.action.stopUserProgram',
 }));
 
+/** Indicates the the stop user program command was sent to the hub. */
+export const hubDidStopUserProgram = createAction(() => ({
+    type: 'hub.action.didStopUserProgram',
+}));
+
+/** Indicates the the stop user program command failed to be sent to the hub. */
+export const hubDidFailToStopUserProgram = createAction(() => ({
+    type: 'hub.action.didFailToStopUserProgram',
+}));
+
+/** Request to send the start repl command to the hub. */
 export const hubStartRepl = createAction((useLegacyDownload: boolean) => ({
     type: 'hub.action.startRepl',
     useLegacyDownload,
 }));
 
+/** Indicates the the start repl command was sent to the hub. */
 export const hubDidStartRepl = createAction(() => ({
     type: 'hub.action.didStartRepl',
 }));
 
+/** Indicates the the start repl command failed to be sent to the hub. */
 export const hubDidFailToStartRepl = createAction(() => ({
     type: 'hub.action.didFailToStartRepl',
 }));

src/hub/reducers.test.ts

@@ -15,7 +15,9 @@ import {
     didProgressDownload,
     didStartDownload,
     hubDidFailToStartRepl,
+    hubDidFailToStopUserProgram,
     hubStartRepl,
+    hubStopUserProgram,
 } from './actions';
 import reducers, { HubRuntimeState } from './reducers';
 
@@ -188,4 +190,22 @@ describe('runtime', () => {
             ).runtime,
         ).toBe(HubRuntimeState.Unknown);
     });
+
+    test('hubStopUserProgram', () => {
+        expect(
+            reducers(
+                { runtime: HubRuntimeState.Running } as State,
+                hubStopUserProgram(),
+            ).runtime,
+        ).toBe(HubRuntimeState.StoppingUserProgram);
+    });
+
+    test('hubDidFailToStopUserProgram', () => {
+        expect(
+            reducers(
+                { runtime: HubRuntimeState.Running } as State,
+                hubDidFailToStopUserProgram(),
+            ).runtime,
+        ).toBe(HubRuntimeState.Unknown);
+    });
 });

src/hub/reducers.ts

@@ -27,7 +27,9 @@ import {
     didProgressDownload,
     didStartDownload,
     hubDidFailToStartRepl,
+    hubDidFailToStopUserProgram,
     hubStartRepl,
+    hubStopUserProgram,
 } from './actions';
 
 /**
@@ -48,6 +50,8 @@ export enum HubRuntimeState {
     Running = 'hub.runtime.running',
     /** Busy starting the REPL. */
     StartingRepl = 'hub.runtime.startingRepl',
+    /** Busy stopping user program. */
+    StoppingUserProgram = 'hub.runtime.stoppingUserProgram',
 }
 
 const runtime: Reducer<HubRuntimeState> = (
@@ -119,6 +123,17 @@ const runtime: Reducer<HubRuntimeState> = (
         return HubRuntimeState.Unknown;
     }
 
+    if (hubStopUserProgram.matches(action)) {
+        return HubRuntimeState.StoppingUserProgram;
+    }
+
+    // NB: hubDidStopUserProgram will trigger user program running flag to clear,
+    // so we don't change state for both to avoid race condition
+
+    if (hubDidFailToStopUserProgram.matches(action)) {
+        return HubRuntimeState.Unknown;
+    }
+
     return state;
 };
 

src/hub/sagas.test.ts

@@ -21,9 +21,11 @@ import {
     didStartDownload,
     downloadAndRun,
     hubDidFailToStartRepl,
+    hubDidFailToStopUserProgram,
     hubDidStartRepl,
+    hubDidStopUserProgram,
     hubStartRepl,
-    stop,
+    hubStopUserProgram,
 } from './actions';
 import hub from './sagas';
 
@@ -136,15 +138,36 @@ describe('hubStartRepl', () => {
     });
 });
 
-test('stop', async () => {
-    const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
+describe('hubStopUserProgram', () => {
+    test('success', async () => {
+        const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
 
-    saga.put(stop());
+        saga.put(hubStopUserProgram());
 
-    const pybricksServiceAction = await saga.take();
-    expect(pybricksServiceAction).toEqual(sendStopUserProgramCommand(0));
+        await expect(saga.take()).resolves.toEqual(sendStopUserProgramCommand(0));
 
-    saga.put(didSendCommand(0));
+        saga.put(didSendCommand(0));
 
-    await saga.end();
+        await expect(saga.take()).resolves.toEqual(hubDidStopUserProgram());
+
+        await saga.end();
+    });
+
+    test('failure due to disconnect', async () => {
+        const saga = new AsyncSaga(hub, { nextMessageId: createCountFunc() });
+
+        saga.put(hubStopUserProgram());
+
+        await expect(saga.take()).resolves.toEqual(sendStopUserProgramCommand(0));
+
+        saga.put(didFailToSendCommand(0, new DOMException('test', 'NetworkError')));
+
+        await expect(saga.take()).resolves.toEqual(
+            alertsShowAlert('ble', 'disconnected'),
+        );
+
+        await expect(saga.take()).resolves.toEqual(hubDidFailToStopUserProgram());
+
+        await saga.end();
+    });
 });

src/hub/sagas.ts

@@ -47,9 +47,11 @@ import {
     didStartDownload,
     downloadAndRun,
     hubDidFailToStartRepl,
+    hubDidFailToStopUserProgram,
     hubDidStartRepl,
+    hubDidStopUserProgram,
     hubStartRepl,
-    stop,
+    hubStopUserProgram,
 } from './actions';
 
 const downloadChunkSize = 100;
@@ -387,27 +389,46 @@ function* handleHubStartRepl(action: ReturnType<typeof hubStartRepl>): Generator
     yield* put(hubDidStartRepl());
 }
 
-function* handleStop(): Generator {
+function* handleStopUserProgram(): Generator {
     const nextMessageId = yield* getContext<() => number>('nextMessageId');
     const id = nextMessageId();
     yield* put(sendStopUserProgramCommand(id));
     // REVISIT: may want to disable button while attempting to send command
     // this would mean didSendStop() and didFailToSendStop() actions here
-    const { failedToSend } = yield* race({
-        sent: take(didSendCommand.when((a) => a.id === id)),
-        failedToSend: take(didFailToSendCommand.when((a) => a.id === id)),
+    const { didFailToSend } = yield* race({
+        didSend: take(didSendCommand.when((a) => a.id === id)),
+        didFailToSend: take(didFailToSendCommand.when((a) => a.id === id)),
     });
-    if (failedToSend) {
-        // TODO: probably want to check error. If hub disconnected, ignore error
-        // otherwise indicate error to user
-        console.error(failedToSend.error);
+
+    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(hubDidFailToStopUserProgram());
+        return;
     }
+
+    yield* put(hubDidStopUserProgram());
 }
 
 export default function* (): Generator {
     yield* takeEvery(downloadAndRun, handleDownloadAndRun);
     yield* takeEvery(hubStartRepl, handleHubStartRepl);
-    yield* takeEvery(stop, handleStop);
+    yield* takeEvery(hubStopUserProgram, handleStopUserProgram);
     // calling stop right after connecting should get the hub into a known state
-    yield* takeEvery(bleDidConnectPybricks, handleStop);
+    yield* takeEvery(bleDidConnectPybricks, handleStopUserProgram);
 }

src/toolbar/buttons/stop/StopButton.test.tsx

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

src/toolbar/buttons/stop/StopButton.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 from 'react';
 import { useDispatch } from 'react-redux';
-import { stop } from '../../../hub/actions';
+import { hubStopUserProgram } from '../../../hub/actions';
 import { HubRuntimeState } from '../../../hub/reducers';
 import { useSelector } from '../../../reducers';
 import ActionButton, { ActionButtonProps } from '../../ActionButton';
@@ -27,7 +27,8 @@ const StopButton: React.VoidFunctionComponent<StopButtonProps> = ({ id }) => {
             tooltip={i18n.translate('tooltip', { key: keyboardShortcut })}
             icon={icon}
             enabled={runtime === HubRuntimeState.Running}
-            onAction={() => dispatch(stop())}
+            showProgress={runtime === HubRuntimeState.StoppingUserProgram}
+            onAction={() => dispatch(hubStopUserProgram())}
         />
     );
 };