I can't actually remember why we have both ShellJSInternalError and exec: internal error, but they mean the same thing: there's a bug in ShellJS code somewhere. These should probably get merged together, so that we can actually keep the exception stack for bugs in exec. This would help immensely in debugging.
While it's true that exec() sometimes crashes unavoidably (like if we overrun the max buffer), it's definitely still helpful to have the stack trace, and it still makes sense to report it as an exception, instead of with shell.error() (that should be reserved for the external command's stderr).
I can't actually remember why we have both
ShellJSInternalErrorandexec: internal error, but they mean the same thing: there's a bug in ShellJS code somewhere. These should probably get merged together, so that we can actually keep the exception stack for bugs inexec. This would help immensely in debugging.While it's true that
exec()sometimes crashes unavoidably (like if we overrun the max buffer), it's definitely still helpful to have the stack trace, and it still makes sense to report it as an exception, instead of withshell.error()(that should be reserved for the external command's stderr).