Exit with error code if compilation fails.

[schmod]

If we are unable to download or compile the underlying jpegtran binary, the build script should be terminated with a non-zero exit code.

This error should cascade upwards, so that npm (and any dependant packages) are aware of the failure.

Thanks to Github's recent network issues, builds in our CI environment (mysteriously) started to fail today, because this package's requests to download the precompiled binaries from Github were failing and unsuccessfully trying to fall back to a manual build (because nasm isn't installed in our CI environment).

[kevva]

This would kill the whole npm install process, right? That's not what we want to do.

[schmod]

As specified in npm's documentation, if a package is specified as an optional dependency, a build failure does not cascade, and cause the entire installation to fail. Optional dependencies exist for this exact reason.

By convention, consumers of optional dependencies should check to see if they installed successfully by doing

try {
  require('optionalDep');
} catch() {
  recoverGracefully();
}

or using a helper such as optional, to be a bit less verbose (which is how imagemin requres this package)

By returning 0 when jpegtran fails to build/install correctly, we are falsely signaling that this module is installed, and safe to use. If we expect downstream packages to depend on this one as an Optional Dependency, there is an established convention and expectation that fatal build errors should block the installation of this package, so that the downstream maintainer can check for the presence/functionality of this module by using a try/catch statement.

If downstream packages depend on this one as a non-optional dependency (many do), it's even more important to mark the build as failed.

[kevva]

@schmod, going to have a look at this. If it works I'll implement it :). I agree that it's better to make the user aware of any potential install failures (we're already logging it when installing). But I remember we used throw or process.exit(1) before and it caused the whole npm install to fail.

[schmod]

Thanks.... If there really are issues where NPM bails if an optional dependency fails, we really need to raise that issue to them, as it's a major bug in their support for optional dependencies. I was unable to reproduce such an issue, but it certainly could have existed in previous versions of NPM...

I'd like to push hard to have this included, because the current behavior makes failed installations extremely difficult to detect and diagnose, particularly in CI environments.