Improve error handling in hooks (rel to #1213)

packages/core/core.test.mts

@@ -186,3 +186,68 @@ describe('createPrompt()', () => {
     await expect(answer).resolves.toEqual('done');
   });
 });
+
+describe('Error handling', () => {
+  it('surface errors in render functions', async () => {
+    const Prompt = () => {
+      throw new Error('Error in render function');
+    };
+
+    const prompt = createPrompt(Prompt);
+    const { answer } = await render(prompt, { message: 'Question' });
+
+    await expect(answer).rejects.toThrowError('Error in render function');
+  });
+
+  it('surface errors in useEffect', async () => {
+    const Prompt = () => {
+      useEffect(() => {
+        throw new Error('Error in useEffect');
+      }, []);
+
+      return '';
+    };
+
+    const prompt = createPrompt(Prompt);
+    const { answer } = await render(prompt, { message: 'Question' });
+
+    await expect(answer).rejects.toThrowError('Error in useEffect');
+  });
+
+  it('surface errors in useEffect cleanup functions', async () => {
+    const Prompt = (config: {}, done: (value: string) => void) => {
+      useEffect(() => {
+        done('done');
+
+        return () => {
+          throw new Error('Error in useEffect cleanup');
+        };
+      }, []);
+
+      return '';
+    };
+
+    const prompt = createPrompt(Prompt);
+    const { answer } = await render(prompt, { message: 'Question' });
+
+    await expect(answer).rejects.toThrowError('Error in useEffect cleanup');
+  });
+
+  it.only('prevent returning promises from useEffect hook', async () => {
+    const Prompt = (config: {}, done: (value: string) => void) => {
+      // @ts-ignore: This is a test
+      useEffect(async () => {
+        done('done');
+      }, []);
+
+      return '';
+    };
+
+    const prompt = createPrompt(Prompt);
+    const { answer } = await render(prompt, { message: 'Question' });
+
+    await expect(answer).rejects.toThrowErrorMatchingInlineSnapshot(
+      '"useEffect return value must be a cleanup function or nothing."'
+    );
+  });
+});

packages/core/src/index.mts

@@ -63,7 +63,12 @@ export function useEffect(cb: () => void | (() => void), depArray: unknown[]): v
   if (hasChanged) {
     hooksEffect.push(() => {
       cleanupHook(_idx);
-      hooksCleanup[_idx] = cb();
+      const cleanFn = cb();
+      console.log('cleanFn', cleanFn);
+      if (cleanFn != null && typeof cleanFn !== 'function') {
+        throw new Error('useEffect return value must be a cleanup function or nothing.');
+      }
+      hooksCleanup[_idx] = cleanFn;
     });
   }
   hooks[_idx] = depArray;
@@ -132,26 +137,34 @@ export function createPrompt<Value, Config extends AsyncPromptConfig>(
     // TODO: we should display a loader while we get the default options.
     const resolvedConfig = await getPromptConfig(config);
 
-    return new Promise((resolve) => {
+    return new Promise((resolve, reject) => {
       const done = (value: Value) => {
-        let len = hooksCleanup.length;
-        while (len--) {
-          cleanupHook(len);
-        }
-        if (context?.clearPromptOnDone) {
-          screen.clean();
-        } else {
-          screen.clearContent();
-        }
-        screen.done();
-
-        // Reset hooks state
-        index = 0;
-        hooks.length = 0;
-        sessionRl = undefined;
-
-        // Finally we resolve our promise
-        resolve(value);
+        // Delay execution to let time to the hookCleanup functions to registers.
+        setImmediate(() => {
+          try {
+            let len = hooksCleanup.length;
+            while (len--) {
+              cleanupHook(len);
+            }
+          } catch (err) {
+            reject(err);
+          }
+
+          if (context?.clearPromptOnDone) {
+            screen.clean();
+          } else {
+            screen.clearContent();
+          }
+          screen.done();
+
+          // Reset hooks state
+          index = 0;
+          hooks.length = 0;
+          sessionRl = undefined;
+
+          // Finally we resolve our promise
+          resolve(value);
+        });
       };
 
       const workLoop = () => {