diff --git a/docs/changelog.md b/docs/changelog.md index 6d92ffa2..d25d59c1 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -14,6 +14,9 @@ Requires libvips v8.14.5 * Remove `sharp.vendor`. +* Ensure all `Error` objects contain a `stack` property. + [#3653](https://github.com/lovell/sharp/issues/3653) + * Make `compression` option of `heif` mandatory to help reduce HEIF vs HEIC confusion. [#3740](https://github.com/lovell/sharp/issues/3740) diff --git a/lib/input.js b/lib/input.js index 0fd4bad6..cafd8fd7 100644 --- a/lib/input.js +++ b/lib/input.js @@ -483,14 +483,27 @@ function _isStreamInput () { * @returns {Promise|Sharp} */ function metadata (callback) { + const stack = Error(); if (is.fn(callback)) { if (this._isStreamInput()) { this.on('finish', () => { this._flattenBufferIn(); - sharp.metadata(this.options, callback); + sharp.metadata(this.options, (err, metadata) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, metadata); + } + }); }); } else { - sharp.metadata(this.options, callback); + sharp.metadata(this.options, (err, metadata) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, metadata); + } + }); } return this; } else { @@ -500,7 +513,7 @@ function metadata (callback) { this._flattenBufferIn(); sharp.metadata(this.options, (err, metadata) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { resolve(metadata); } @@ -516,7 +529,7 @@ function metadata (callback) { return new Promise((resolve, reject) => { sharp.metadata(this.options, (err, metadata) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { resolve(metadata); } @@ -572,14 +585,27 @@ function metadata (callback) { * @returns {Promise} */ function stats (callback) { + const stack = Error(); if (is.fn(callback)) { if (this._isStreamInput()) { this.on('finish', () => { this._flattenBufferIn(); - sharp.stats(this.options, callback); + sharp.stats(this.options, (err, stats) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, stats); + } + }); }); } else { - sharp.stats(this.options, callback); + sharp.stats(this.options, (err, stats) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, stats); + } + }); } return this; } else { @@ -589,7 +615,7 @@ function stats (callback) { this._flattenBufferIn(); sharp.stats(this.options, (err, stats) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { resolve(stats); } @@ -600,7 +626,7 @@ function stats (callback) { return new Promise((resolve, reject) => { sharp.stats(this.options, (err, stats) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { resolve(stats); } diff --git a/lib/is.js b/lib/is.js index f1f390bc..a63cb206 100644 --- a/lib/is.js +++ b/lib/is.js @@ -137,6 +137,19 @@ const invalidParameterError = function (name, expected, actual) { ); }; +/** + * Ensures an Error from C++ contains a JS stack. + * + * @param {Error} native - Error with message from C++. + * @param {Error} context - Error with stack from JS. + * @returns {Error} Error with message and stack. + * @private + */ +const nativeError = function (native, context) { + context.message = native.message; + return context; +}; + module.exports = { defined, object, @@ -151,5 +164,6 @@ module.exports = { integer, inRange, inArray, - invalidParameterError + invalidParameterError, + nativeError }; diff --git a/lib/output.js b/lib/output.js index 1ab36aa9..3c6371a5 100644 --- a/lib/output.js +++ b/lib/output.js @@ -86,7 +86,8 @@ function toFile (fileOut, callback) { } } else { this.options.fileOut = fileOut; - return this._pipeline(callback); + const stack = Error(); + return this._pipeline(callback, stack); } return this; } @@ -157,7 +158,8 @@ function toBuffer (options, callback) { this.options.resolveWithObject = false; } this.options.fileOut = ''; - return this._pipeline(is.fn(options) ? options : callback); + const stack = Error(); + return this._pipeline(is.fn(options) ? options : callback, stack); } /** @@ -1282,7 +1284,8 @@ function _read () { /* istanbul ignore else */ if (!this.options.streamOut) { this.options.streamOut = true; - this._pipeline(); + const stack = Error(); + this._pipeline(undefined, stack); } } @@ -1291,18 +1294,30 @@ function _read () { * Supports callback, stream and promise variants * @private */ -function _pipeline (callback) { +function _pipeline (callback, stack) { if (typeof callback === 'function') { // output=file/buffer if (this._isStreamInput()) { // output=file/buffer, input=stream this.on('finish', () => { this._flattenBufferIn(); - sharp.pipeline(this.options, callback); + sharp.pipeline(this.options, (err, data, info) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, data, info); + } + }); }); } else { // output=file/buffer, input=file/buffer - sharp.pipeline(this.options, callback); + sharp.pipeline(this.options, (err, data, info) => { + if (err) { + callback(is.nativeError(err, stack)); + } else { + callback(null, data, info); + } + }); } return this; } else if (this.options.streamOut) { @@ -1313,7 +1328,7 @@ function _pipeline (callback) { this._flattenBufferIn(); sharp.pipeline(this.options, (err, data, info) => { if (err) { - this.emit('error', err); + this.emit('error', is.nativeError(err, stack)); } else { this.emit('info', info); this.push(data); @@ -1329,7 +1344,7 @@ function _pipeline (callback) { // output=stream, input=file/buffer sharp.pipeline(this.options, (err, data, info) => { if (err) { - this.emit('error', err); + this.emit('error', is.nativeError(err, stack)); } else { this.emit('info', info); this.push(data); @@ -1348,7 +1363,7 @@ function _pipeline (callback) { this._flattenBufferIn(); sharp.pipeline(this.options, (err, data, info) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { if (this.options.resolveWithObject) { resolve({ data, info }); @@ -1364,7 +1379,7 @@ function _pipeline (callback) { return new Promise((resolve, reject) => { sharp.pipeline(this.options, (err, data, info) => { if (err) { - reject(err); + reject(is.nativeError(err, stack)); } else { if (this.options.resolveWithObject) { resolve({ data, info }); diff --git a/test/unit/io.js b/test/unit/io.js index 485d07fc..bdb4c9ce 100644 --- a/test/unit/io.js +++ b/test/unit/io.js @@ -459,27 +459,29 @@ describe('Input/output', function () { }); }); - it('Fail when input is invalid Buffer', function (done) { - sharp(Buffer.from([0x1, 0x2, 0x3, 0x4])).toBuffer().then(function () { - assert(false); - done(); - }).catch(function (err) { - assert(err instanceof Error); - assert.strictEqual('Input buffer contains unsupported image format', err.message); - done(); - }); - }); + it('Fail when input is invalid Buffer', async () => + assert.rejects( + () => sharp(Buffer.from([0x1, 0x2, 0x3, 0x4])).toBuffer(), + (err) => { + assert.strictEqual(err.message, 'Input buffer contains unsupported image format'); + assert(err.stack.includes('at Sharp.toBuffer')); + assert(err.stack.includes(__filename)); + return true; + } + ) + ); - it('Fail when input file path is missing', function (done) { - sharp('does-not-exist').toBuffer().then(function () { - assert(false); - done(); - }).catch(function (err) { - assert(err instanceof Error); - assert.strictEqual('Input file is missing: does-not-exist', err.message); - done(); - }); - }); + it('Fail when input file path is missing', async () => + assert.rejects( + () => sharp('does-not-exist').toFile('fail'), + (err) => { + assert.strictEqual(err.message, 'Input file is missing: does-not-exist'); + assert(err.stack.includes('at Sharp.toFile')); + assert(err.stack.includes(__filename)); + return true; + } + ) + ); describe('Fail for unsupported input', function () { it('Undefined', function () { diff --git a/test/unit/metadata.js b/test/unit/metadata.js index a16cc89e..39f37ae5 100644 --- a/test/unit/metadata.js +++ b/test/unit/metadata.js @@ -395,13 +395,27 @@ describe('Image metadata', function () { }); }); - it('Non-existent file in, Promise out', function (done) { - sharp('fail').metadata().then(function (metadata) { - throw new Error('Non-existent file'); - }, function (err) { - assert.ok(!!err); - done(); - }); + it('Non-existent file in, Promise out', async () => + assert.rejects( + () => sharp('fail').metadata(), + (err) => { + assert.strictEqual(err.message, 'Input file is missing: fail'); + assert(err.stack.includes('at Sharp.metadata')); + assert(err.stack.includes(__filename)); + return true; + } + ) + ); + + it('Invalid stream in, callback out', (done) => { + fs.createReadStream(__filename).pipe( + sharp().metadata((err) => { + assert.strictEqual(err.message, 'Input buffer contains unsupported image format'); + assert(err.stack.includes('at Sharp.metadata')); + assert(err.stack.includes(__filename)); + done(); + }) + ); }); it('Stream in, Promise out', function (done) { diff --git a/test/unit/stats.js b/test/unit/stats.js index 72e42fc7..22895ac8 100644 --- a/test/unit/stats.js +++ b/test/unit/stats.js @@ -690,11 +690,25 @@ describe('Image Stats', function () { it('File input with corrupt header fails gracefully', function (done) { sharp(fixtures.inputJpgWithCorruptHeader) .stats(function (err) { - assert.strictEqual(true, !!err); + assert(err.message.includes('Input file has corrupt header')); + assert(err.stack.includes('at Sharp.stats')); + assert(err.stack.includes(__filename)); done(); }); }); + it('Stream input with corrupt header fails gracefully', function (done) { + fs.createReadStream(fixtures.inputJpgWithCorruptHeader).pipe( + sharp() + .stats(function (err) { + assert(err.message.includes('Input buffer has corrupt header')); + assert(err.stack.includes('at Sharp.stats')); + assert(err.stack.includes(__filename)); + done(); + }) + ); + }); + it('File input with corrupt header fails gracefully, Promise out', function () { return sharp(fixtures.inputJpgWithCorruptHeader) .stats().then(function (stats) {