vadimdemedes/ink

Uncaught exceptions always result in stacktrace

mattrothenberg opened this issue · 4 comments

Hello!

I'm trying to understand why an ink app rendered by the commander CLI library always seems to barf out stacktraces when exceptions occur, even when the offending code is:

  • Wrapped in a React Error Boundary
  • Executed when NODE_ENV=production

Effectively, in the reproduction repository below, I would expect ink to handle the throw that occurs 2 seconds after the component mounts, causing the parent ErrorBoundary to occur and preventing the stack trace from leaking out.

CleanShot 2023-03-01 at 15 16 29@2x

It's very possible that I've either set this demo up incorrectly, or I have a misunderstanding of how exceptions work in Node 😅

In any event, I'd welcome advice for handling the (inevitable) exceptions that occur in client-side React/Ink code, and how I can mitigate showing a potentially massive stack trace to the end users of my CLI.

Reproduction repository: https://github.com/mattrothenberg/ink-cmd-repro

Please let me know how I can help elaborate!

Hey Matt! I took a look at your repository and it is actually expected that React's error boundary doesn't catch the error. It's wrapped into a setTimeout, which will essentially throw an error in the top-level / global scope and not inside useEffecthttps://github.com/mattrothenberg/ink-cmd-repro/blob/main/src/lib.tsx#L8.

What's happening there is basically this:

const run = () => {
  setTimeout(() => {
    throw new Error('Oh no')
  }, 2000);
};

try {
  run()
} catch (error) {
  // Will not catch error
}

Thanks for the quick reply! That sort of makes sense.

I guess I'm generally confused, then, how exceptions thrown outside of a setTimeout (that was used here for illustration purposes) would propagate to an error boundary? I'm able to reproduce this in other scenarios too, and ultimately I'm just trying to mitigate against the inevitable exception being thrown and leaking a really ugly stack trace to my end users.

Edit: The React docs actually explain this pretty clearly. I'm being a dummy. Thanks again for looking into this!

I guess I'm generally confused, then, how exceptions thrown outside of a setTimeout (that was used here for illustration purposes) would propagate to an error boundary?

If we look at the example code above, as soon as you remove setTimeout, try..catch will catch the Error, which is essentially what an error boundary does. Error boundary only catches errors that happen during rendering, i.e. synchronous errors while executing component functions, hooks and such.

Error boundary doesn't catch uncaught exceptions or unhandled promise rejections. If you want to guard against these types of errors too, you'd need to catch these separately.

Here's an example I cooked up of how it could look:

import React from 'react';
import {render, Text} from 'ink';

class AdvancedErrorBoundary extends React.Component {
	state = {
		isCrashed: false
	};

	static getDerivedStateFromError(error) {
		return {
			isCrashed: true
		};
	}

	render() {
		if (this.state.isCrashed) {
			return <Text color="red">Oh no, app crashed</Text>;
		}

		return this.props.children;
	}

	componentDidCatch(error) {
		// Errored while rendering components
	}

	componentDidMount() {
		process.setUncaughtExceptionCaptureCallback(this.crashed)
	}

	componentWillUnmount() {
		process.setUncaughtExceptionCaptureCallback(null)
	}

	crashed = (...args) => {
		this.setState({
			isCrashed: true
		});

		process.exitCode = 1;
	};
}

render(
	<AdvancedErrorBoundary>
		<MyApp />
	</AdvancedErrorBoundary>,
);

This would guard against uncaught exception like this:

setTimeout(() => {
	throw new Error('Trigger uncaughtException');
}, 1000);

And unhandled promise rejections:

const fail = async () => {
	throw new Error('Trigger unhandledRejection');
};

fail();

This is perfect @vadimdemedes, thanks so much for your help here.