Ensure all Error objects contain a stack prop #3653

This commit is contained in:
Lovell Fuller 2023-10-11 14:59:21 +01:00
parent 68fa84ef6f
commit 47e76c9981
7 changed files with 135 additions and 47 deletions

View File

@ -14,6 +14,9 @@ Requires libvips v8.14.5
* Remove `sharp.vendor`. * 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. * Make `compression` option of `heif` mandatory to help reduce HEIF vs HEIC confusion.
[#3740](https://github.com/lovell/sharp/issues/3740) [#3740](https://github.com/lovell/sharp/issues/3740)

View File

@ -483,14 +483,27 @@ function _isStreamInput () {
* @returns {Promise<Object>|Sharp} * @returns {Promise<Object>|Sharp}
*/ */
function metadata (callback) { function metadata (callback) {
const stack = Error();
if (is.fn(callback)) { if (is.fn(callback)) {
if (this._isStreamInput()) { if (this._isStreamInput()) {
this.on('finish', () => { this.on('finish', () => {
this._flattenBufferIn(); 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 { } 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; return this;
} else { } else {
@ -500,7 +513,7 @@ function metadata (callback) {
this._flattenBufferIn(); this._flattenBufferIn();
sharp.metadata(this.options, (err, metadata) => { sharp.metadata(this.options, (err, metadata) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
resolve(metadata); resolve(metadata);
} }
@ -516,7 +529,7 @@ function metadata (callback) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
sharp.metadata(this.options, (err, metadata) => { sharp.metadata(this.options, (err, metadata) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
resolve(metadata); resolve(metadata);
} }
@ -572,14 +585,27 @@ function metadata (callback) {
* @returns {Promise<Object>} * @returns {Promise<Object>}
*/ */
function stats (callback) { function stats (callback) {
const stack = Error();
if (is.fn(callback)) { if (is.fn(callback)) {
if (this._isStreamInput()) { if (this._isStreamInput()) {
this.on('finish', () => { this.on('finish', () => {
this._flattenBufferIn(); 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 { } 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; return this;
} else { } else {
@ -589,7 +615,7 @@ function stats (callback) {
this._flattenBufferIn(); this._flattenBufferIn();
sharp.stats(this.options, (err, stats) => { sharp.stats(this.options, (err, stats) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
resolve(stats); resolve(stats);
} }
@ -600,7 +626,7 @@ function stats (callback) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
sharp.stats(this.options, (err, stats) => { sharp.stats(this.options, (err, stats) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
resolve(stats); resolve(stats);
} }

View File

@ -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 = { module.exports = {
defined, defined,
object, object,
@ -151,5 +164,6 @@ module.exports = {
integer, integer,
inRange, inRange,
inArray, inArray,
invalidParameterError invalidParameterError,
nativeError
}; };

View File

@ -86,7 +86,8 @@ function toFile (fileOut, callback) {
} }
} else { } else {
this.options.fileOut = fileOut; this.options.fileOut = fileOut;
return this._pipeline(callback); const stack = Error();
return this._pipeline(callback, stack);
} }
return this; return this;
} }
@ -157,7 +158,8 @@ function toBuffer (options, callback) {
this.options.resolveWithObject = false; this.options.resolveWithObject = false;
} }
this.options.fileOut = ''; 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 */ /* istanbul ignore else */
if (!this.options.streamOut) { if (!this.options.streamOut) {
this.options.streamOut = true; 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 * Supports callback, stream and promise variants
* @private * @private
*/ */
function _pipeline (callback) { function _pipeline (callback, stack) {
if (typeof callback === 'function') { if (typeof callback === 'function') {
// output=file/buffer // output=file/buffer
if (this._isStreamInput()) { if (this._isStreamInput()) {
// output=file/buffer, input=stream // output=file/buffer, input=stream
this.on('finish', () => { this.on('finish', () => {
this._flattenBufferIn(); 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 { } else {
// output=file/buffer, input=file/buffer // 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; return this;
} else if (this.options.streamOut) { } else if (this.options.streamOut) {
@ -1313,7 +1328,7 @@ function _pipeline (callback) {
this._flattenBufferIn(); this._flattenBufferIn();
sharp.pipeline(this.options, (err, data, info) => { sharp.pipeline(this.options, (err, data, info) => {
if (err) { if (err) {
this.emit('error', err); this.emit('error', is.nativeError(err, stack));
} else { } else {
this.emit('info', info); this.emit('info', info);
this.push(data); this.push(data);
@ -1329,7 +1344,7 @@ function _pipeline (callback) {
// output=stream, input=file/buffer // output=stream, input=file/buffer
sharp.pipeline(this.options, (err, data, info) => { sharp.pipeline(this.options, (err, data, info) => {
if (err) { if (err) {
this.emit('error', err); this.emit('error', is.nativeError(err, stack));
} else { } else {
this.emit('info', info); this.emit('info', info);
this.push(data); this.push(data);
@ -1348,7 +1363,7 @@ function _pipeline (callback) {
this._flattenBufferIn(); this._flattenBufferIn();
sharp.pipeline(this.options, (err, data, info) => { sharp.pipeline(this.options, (err, data, info) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
if (this.options.resolveWithObject) { if (this.options.resolveWithObject) {
resolve({ data, info }); resolve({ data, info });
@ -1364,7 +1379,7 @@ function _pipeline (callback) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
sharp.pipeline(this.options, (err, data, info) => { sharp.pipeline(this.options, (err, data, info) => {
if (err) { if (err) {
reject(err); reject(is.nativeError(err, stack));
} else { } else {
if (this.options.resolveWithObject) { if (this.options.resolveWithObject) {
resolve({ data, info }); resolve({ data, info });

View File

@ -459,27 +459,29 @@ describe('Input/output', function () {
}); });
}); });
it('Fail when input is invalid Buffer', function (done) { it('Fail when input is invalid Buffer', async () =>
sharp(Buffer.from([0x1, 0x2, 0x3, 0x4])).toBuffer().then(function () { assert.rejects(
assert(false); () => sharp(Buffer.from([0x1, 0x2, 0x3, 0x4])).toBuffer(),
done(); (err) => {
}).catch(function (err) { assert.strictEqual(err.message, 'Input buffer contains unsupported image format');
assert(err instanceof Error); assert(err.stack.includes('at Sharp.toBuffer'));
assert.strictEqual('Input buffer contains unsupported image format', err.message); assert(err.stack.includes(__filename));
done(); return true;
}); }
}); )
);
it('Fail when input file path is missing', function (done) { it('Fail when input file path is missing', async () =>
sharp('does-not-exist').toBuffer().then(function () { assert.rejects(
assert(false); () => sharp('does-not-exist').toFile('fail'),
done(); (err) => {
}).catch(function (err) { assert.strictEqual(err.message, 'Input file is missing: does-not-exist');
assert(err instanceof Error); assert(err.stack.includes('at Sharp.toFile'));
assert.strictEqual('Input file is missing: does-not-exist', err.message); assert(err.stack.includes(__filename));
done(); return true;
}); }
}); )
);
describe('Fail for unsupported input', function () { describe('Fail for unsupported input', function () {
it('Undefined', function () { it('Undefined', function () {

View File

@ -395,13 +395,27 @@ describe('Image metadata', function () {
}); });
}); });
it('Non-existent file in, Promise out', function (done) { it('Non-existent file in, Promise out', async () =>
sharp('fail').metadata().then(function (metadata) { assert.rejects(
throw new Error('Non-existent file'); () => sharp('fail').metadata(),
}, function (err) { (err) => {
assert.ok(!!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(); done();
}); })
);
}); });
it('Stream in, Promise out', function (done) { it('Stream in, Promise out', function (done) {

View File

@ -690,11 +690,25 @@ describe('Image Stats', function () {
it('File input with corrupt header fails gracefully', function (done) { it('File input with corrupt header fails gracefully', function (done) {
sharp(fixtures.inputJpgWithCorruptHeader) sharp(fixtures.inputJpgWithCorruptHeader)
.stats(function (err) { .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(); 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 () { it('File input with corrupt header fails gracefully, Promise out', function () {
return sharp(fixtures.inputJpgWithCorruptHeader) return sharp(fixtures.inputJpgWithCorruptHeader)
.stats().then(function (stats) { .stats().then(function (stats) {