refactor: modernize MulterError to ES6 class#1357
refactor: modernize MulterError to ES6 class#1357Phillip9587 wants to merge 1 commit intoexpressjs:mainfrom
Conversation
| } | ||
|
|
||
| function MulterError (code, field) { | ||
| Error.captureStackTrace(this, this.constructor) |
There was a problem hiding this comment.
Is not needed anymore as the stack trace is automatically captured by the constructors super call.
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the MulterError implementation by converting it from a function constructor pattern to an ES6 class syntax, eliminating the need for util.inherits() and adopting contemporary JavaScript patterns.
- Converts function constructor to ES6 class extending Error
- Removes util.inherits() dependency and util import
- Updates variable declaration from var to const for errorMessages
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (field) this.field = field | ||
| class MulterError extends Error { | ||
| constructor (code, field) { | ||
| super(errorMessages[code]) |
There was a problem hiding this comment.
The Error.captureStackTrace() call was removed but not replaced. In ES6 classes extending Error, you should call Error.captureStackTrace(this, MulterError) after super() to maintain proper stack trace behavior.
| super(errorMessages[code]) | |
| super(errorMessages[code]) | |
| Error.captureStackTrace(this, MulterError) |
There was a problem hiding this comment.
I ran a short test using the following script:
// ORIGINAL IMPLEMENTATION
function MulterErrorOriginal(message) {
Error.captureStackTrace(this, this.constructor);
this.name = this.constructor.name;
this.message = message;
}
require('util').inherits(MulterErrorOriginal, Error);
// NEW ES6 CLASS IMPLEMENTATION (with Error.captureStackTrace)
class MulterErrorNew extends Error {
constructor(message) {
super(message);
this.name = this.constructor.name;
Error.captureStackTrace(this, MulterErrorNew);
}
}
// NEW ES6 CLASS IMPLEMENTATION (without Error.captureStackTrace)
class MulterErrorNewNoCapture extends Error {
constructor(message) {
super(message);
this.name = this.constructor.name;
}
}
function testErrorImplementation(ErrorClass, name) {
console.log(`\n${'='.repeat(50)}`);
console.log(`${name}`);
console.log(`${'='.repeat(50)}`);
// Basic properties test
const err = new ErrorClass('LIMIT_FILE_SIZE', 'avatar');
console.log('Properties:');
console.log(' name:', err.name);
console.log(' instanceof Error:', err instanceof Error);
console.log(' instanceof ErrorClass:', err instanceof ErrorClass);
try {
throw new ErrorClass('LIMIT_FILE_COUNT');
} catch (error) {
console.log(error.stack);
}
}
console.log('Node.js version:', process.version);
console.log('Comparing MulterError implementations...');
// Test all three implementations
testErrorImplementation(MulterErrorOriginal, 'ORIGINAL (function + util.inherits)');
testErrorImplementation(MulterErrorNew, 'NEW ES6 CLASS (with Error.captureStackTrace)');
testErrorImplementation(MulterErrorNewNoCapture, 'NEW ES6 CLASS (without Error.captureStackTrace)');As expected all outputs are the same so this comment is wrong:
Node.js version: v10.16.3
Comparing MulterError implementations...
==================================================
ORIGINAL (function + util.inherits)
==================================================
Properties:
name: MulterErrorOriginal
instanceof Error: true
instanceof ErrorClass: true
MulterErrorOriginal: LIMIT_FILE_COUNT
at testErrorImplementation (/home/pbarta/repos/expressjs/multer/compare-implementations.js:47:11)
at Object.<anonymous> (/home/pbarta/repos/expressjs/multer/compare-implementations.js:57:1)
at Module._compile (internal/modules/cjs/loader.js:778:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
at Module.load (internal/modules/cjs/loader.js:653:32)
at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
at Function.Module._load (internal/modules/cjs/loader.js:585:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
at startup (internal/bootstrap/node.js:283:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)
==================================================
NEW ES6 CLASS (with Error.captureStackTrace)
==================================================
Properties:
name: MulterErrorNew
instanceof Error: true
instanceof ErrorClass: true
MulterErrorNew: LIMIT_FILE_COUNT
at testErrorImplementation (/home/pbarta/repos/expressjs/multer/compare-implementations.js:47:11)
at Object.<anonymous> (/home/pbarta/repos/expressjs/multer/compare-implementations.js:58:1)
at Module._compile (internal/modules/cjs/loader.js:778:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
at Module.load (internal/modules/cjs/loader.js:653:32)
at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
at Function.Module._load (internal/modules/cjs/loader.js:585:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
at startup (internal/bootstrap/node.js:283:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)
==================================================
NEW ES6 CLASS (without Error.captureStackTrace)
==================================================
Properties:
name: MulterErrorNewNoCapture
instanceof Error: true
instanceof ErrorClass: true
MulterErrorNewNoCapture: LIMIT_FILE_COUNT
at testErrorImplementation (/home/pbarta/repos/expressjs/multer/compare-implementations.js:47:11)
at Object.<anonymous> (/home/pbarta/repos/expressjs/multer/compare-implementations.js:59:1)
at Module._compile (internal/modules/cjs/loader.js:778:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
at Module.load (internal/modules/cjs/loader.js:653:32)
at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
at Function.Module._load (internal/modules/cjs/loader.js:585:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
at startup (internal/bootstrap/node.js:283:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:622:3)
This PR modernizes the MulterError implementation by converting it from a function constructor with the legacy util.inherits() pattern to an ES6 class.