ChildProcess: account for a system error when launching a process

[Hi-Angel]

Description of the change

When a non-existing command is attempted to run, both status and signal will be null, which leads to ChildProcess module crashing. Obviously, this is incorrect behavior.

The problem is more general than that though: any system error that would result in failing to run a process would result in the module crash.

Fix that by checking for such case.

Fixes: #65

---

Checklist:

• Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
• Linked any existing issues or proposals that this pull request should close
• Updated or added relevant documentation
• Added a test for the contribution (if applicable)

[garyb]

Nice! 👍

[Hi-Angel]

Thanks!

CI fails seems unrelated, perhaps aff module needs to be declared in configs… I didn't manage to build the project locally too, been testing my changes by copying into a separate project.

[Hi-Angel]

Managed to build it locally, and yes, in bower.json there was aff dep missing.

I fixed it in a separate commit, please tell if you want it to be sent in a separate PR.

[Hi-Angel]

I am not exactly sure what purs-tidy complains about…

I tried moving if to the next line and running purs-tidy locally, but it still complains.

I mean, the only other possible change that I see is making entire if-then-else branch a single line…

[garyb]

Ah yeah, purs-tidy won't like that if/then formatting - you can just run it to format the code rather than using check, but it probably won't look great with what it does either. I'll make a suggestion that it should be happy with.

[garyb]

Suggested change

	_, _ -> if isJust $ toMaybe r.error
	then BySysError
	else unsafeCrashWith $ "Impossible: `spawnSync` child process neither exited nor was killed."
	_, _ ->
	if isJust (toMaybe r.error) then
	BySysError
	else
	unsafeCrashWith "Impossible: `spawnSync` child process neither exited nor was killed."

[garyb]

Or option 2:

Suggested change

	_, _ -> if isJust $ toMaybe r.error
	then BySysError
	else unsafeCrashWith $ "Impossible: `spawnSync` child process neither exited nor was killed."
	_, _
	| isJust (toMaybe r.error) -> BySysError
	| otherwise -> unsafeCrashWith "Impossible: `spawnSync` child process neither exited nor was killed."

[Hi-Angel]

Oh, I like this one! Let me try this

[garyb]

Ah sorry, didn't see this message until now 😄 I think either way is fine though stylistically, and they should compile to pretty much the same JS.

[Hi-Angel]

It's okay and thank you!

[Hi-Angel]

Thank you, done!

[garyb]

Thanks!

[garyb]

Now released as v12.0.0 - even though this was a bug fix, the introduction of BySysError makes it a breaking change, as if there's existing code out there pattern matching on Exit it would no longer be an exhaustive match. 🙂