mirror of
https://github.com/goldbergyoni/nodebestpractices.git
synced 2025-10-30 00:57:04 +08:00
Merge pull request #116 from AllenFang/master
prettify and unify the syntax
This commit is contained in:
@ -722,7 +722,8 @@ This repository is being kept up to date thanks to the help from the community.
|
||||
🌻 [Robert Manolea](https://github.com/pupix),
|
||||
🌻 [Xavier Ho](https://github.com/spaxe),
|
||||
🌻 [Aaron Arney](https://github.com/ocularrhythm),
|
||||
🌻 [Jan Charles Maghirang Adona](https://github.com/septa97)
|
||||
🌻 [Jan Charles Maghirang Adona](https://github.com/septa97),
|
||||
🌻 [Allen Fang](https://github.com/AllenFang)
|
||||
|
||||
|
||||
|
||||
|
||||
@ -11,8 +11,7 @@ Typically, most of modern Node.JS/Express application code runs within promises
|
||||
### Code example: these errors will not get caught by any error handler (except unhandledRejection)
|
||||
|
||||
```javascript
|
||||
DAL.getUserById(1).then((johnSnow) =>
|
||||
{
|
||||
DAL.getUserById(1).then((johnSnow) => {
|
||||
//this error will just vanish
|
||||
if(johnSnow.isAlive == false)
|
||||
throw new Error('ahhhh');
|
||||
@ -23,11 +22,11 @@ DAL.getUserById(1).then((johnSnow) =>
|
||||
### Code example: Catching unresolved and rejected promises
|
||||
|
||||
```javascript
|
||||
process.on('unhandledRejection', function (reason, p) {
|
||||
process.on('unhandledRejection', (reason, p) => {
|
||||
//I just caught an unhandled promise rejection, since we already have fallback handler for unhandled errors (see below), let throw and let him handle that
|
||||
throw reason;
|
||||
});
|
||||
process.on('uncaughtException', function (error) {
|
||||
process.on('uncaughtException', (error) => {
|
||||
//I just received an error that was never handled, time to handle it and then decide whether a restart is needed
|
||||
errorManagement.handler.handleError(error);
|
||||
if (!errorManagement.handler.isTrustedError(error))
|
||||
@ -42,16 +41,16 @@ process.on('uncaughtException', function (error) {
|
||||
> Let’s test your understanding. Which of the following would you expect to print an error to the console?
|
||||
|
||||
```javascript
|
||||
Promise.resolve(‘promised value’).then(function() {
|
||||
throw new Error(‘error’);
|
||||
Promise.resolve(‘promised value’).then(() => {
|
||||
throw new Error(‘error’);
|
||||
});
|
||||
|
||||
Promise.reject(‘error value’).catch(function() {
|
||||
throw new Error(‘error’);
|
||||
Promise.reject(‘error value’).catch(() => {
|
||||
throw new Error(‘error’);
|
||||
});
|
||||
|
||||
new Promise(function(resolve, reject) {
|
||||
throw new Error(‘error’);
|
||||
new Promise((resolve, reject) => {
|
||||
throw new Error(‘error’);
|
||||
});
|
||||
```
|
||||
|
||||
|
||||
@ -18,7 +18,7 @@ DB.addDocument(newCustomer, (error, result) => {
|
||||
|
||||
//API route code, we catch both sync and async errors and forward to the middleware
|
||||
try {
|
||||
customerService.addNew(req.body).then(function (result) {
|
||||
customerService.addNew(req.body).then((result) => {
|
||||
res.status(200).json(result);
|
||||
}).catch((error) => {
|
||||
next(error)
|
||||
@ -29,7 +29,7 @@ catch (error) {
|
||||
}
|
||||
|
||||
//Error handling middleware, we delegate the handling to the centralized error handler
|
||||
app.use(function (err, req, res, next) {
|
||||
app.use((err, req, res, next) => {
|
||||
errorHandler.handleError(err).then((isOperationalError) => {
|
||||
if (!isOperationalError)
|
||||
next(err);
|
||||
@ -47,14 +47,14 @@ function errorHandler(){
|
||||
this.handleError = function (error) {
|
||||
return logger.logError(err).then(sendMailToAdminIfCritical).then(saveInOpsQueueIfCritical).then(determineIfOperationalError);
|
||||
}
|
||||
|
||||
}
|
||||
```
|
||||
|
||||
### Code Example – Anti Pattern: handling errors within the middleware
|
||||
|
||||
```javascript
|
||||
//middleware handling the error directly, who will handle Cron jobs and testing errors?
|
||||
app.use(function (err, req, res, next) {
|
||||
app.use((err, req, res, next) => {
|
||||
logger.logError(err);
|
||||
if(err.severity == errors.high)
|
||||
mailer.sendMail(configuration.adminMail, "Critical error occured", err);
|
||||
|
||||
@ -13,22 +13,21 @@ Somewhere within your code, an error handler object is responsible for deciding
|
||||
//deciding whether to crash when an uncaught exception arrives
|
||||
//Assuming developers mark known operational errors with error.isOperational=true, read best practice #3
|
||||
process.on('uncaughtException', function(error) {
|
||||
errorManagement.handler.handleError(error);
|
||||
if(!errorManagement.handler.isTrustedError(error))
|
||||
process.exit(1)
|
||||
errorManagement.handler.handleError(error);
|
||||
if(!errorManagement.handler.isTrustedError(error))
|
||||
process.exit(1)
|
||||
});
|
||||
|
||||
|
||||
//centralized error handler encapsulates error-handling related logic
|
||||
function errorHandler(){
|
||||
this.handleError = function (error) {
|
||||
return logger.logError(err).then(sendMailToAdminIfCritical).then(saveInOpsQueueIfCritical).then(determineIfOperationalError);
|
||||
}
|
||||
this.handleError = function (error) {
|
||||
return logger.logError(err).then(sendMailToAdminIfCritical).then(saveInOpsQueueIfCritical).then(determineIfOperationalError);
|
||||
}
|
||||
|
||||
this.isTrustedError = function(error)
|
||||
{
|
||||
return error.isOperational;
|
||||
}
|
||||
this.isTrustedError = function (error) {
|
||||
return error.isOperational;
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
|
||||
@ -11,11 +11,11 @@ Testing ‘happy’ paths is no better than testing failures. Good testing code
|
||||
|
||||
```javascript
|
||||
describe("Facebook chat", () => {
|
||||
it("Notifies on new chat message", () => {
|
||||
var chatService = new chatService();
|
||||
chatService.participants = getDisconnectedParticipants();
|
||||
expect(chatService.sendMessage.bind({message: "Hi"})).to.throw(ConnectionError);
|
||||
});
|
||||
it("Notifies on new chat message", () => {
|
||||
var chatService = new chatService();
|
||||
chatService.participants = getDisconnectedParticipants();
|
||||
expect(chatService.sendMessage.bind({ message: "Hi" })).to.throw(ConnectionError);
|
||||
});
|
||||
});
|
||||
|
||||
```
|
||||
@ -24,14 +24,19 @@ describe("Facebook chat", () => {
|
||||
|
||||
```javascript
|
||||
it("Creates new Facebook group", function (done) {
|
||||
var invalidGroupInfo = {};
|
||||
httpRequest({method: 'POST', uri: "facebook.com/api/groups", resolveWithFullResponse: true, body: invalidGroupInfo, json: true
|
||||
}).then((response) => {
|
||||
//oh no if we reached here than no exception was thrown
|
||||
}).catch(function (response) {
|
||||
expect(400).to.equal(response.statusCode);
|
||||
done();
|
||||
});
|
||||
});
|
||||
var invalidGroupInfo = {};
|
||||
httpRequest({
|
||||
method: 'POST',
|
||||
uri: "facebook.com/api/groups",
|
||||
resolveWithFullResponse: true,
|
||||
body: invalidGroupInfo,
|
||||
json: true
|
||||
}).then((response) => {
|
||||
//oh no if we reached here than no exception was thrown
|
||||
}).catch(function (response) {
|
||||
expect(400).to.equal(response.statusCode);
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
```
|
||||
@ -14,12 +14,12 @@ We all loovve console.log but obviously a reputable and persisted Logger like [W
|
||||
```javascript
|
||||
//your centralized logger object
|
||||
var logger = new winston.Logger({
|
||||
level: 'info',
|
||||
transports: [
|
||||
new (winston.transports.Console)(),
|
||||
new (winston.transports.File)({ filename: 'somefile.log' })
|
||||
]
|
||||
});
|
||||
level: 'info',
|
||||
transports: [
|
||||
new (winston.transports.Console)(),
|
||||
new (winston.transports.File)({ filename: 'somefile.log' })
|
||||
]
|
||||
});
|
||||
|
||||
//custom code somewhere using the logger
|
||||
logger.log('info', 'Test Log Message with some parameter %s', 'some parameter', { anything: 'This is metadata' });
|
||||
@ -30,15 +30,19 @@ logger.log('info', 'Test Log Message with some parameter %s', 'some parameter',
|
||||
|
||||
```javascript
|
||||
var options = {
|
||||
from: new Date - 24 * 60 * 60 * 1000, until: new Date, limit: 10, start: 0,
|
||||
order: 'desc', fields: ['message']
|
||||
};
|
||||
from: new Date - 24 * 60 * 60 * 1000,
|
||||
until: new Date,
|
||||
limit: 10,
|
||||
start: 0,
|
||||
order: 'desc',
|
||||
fields: ['message']
|
||||
};
|
||||
|
||||
|
||||
// Find items logged between today and yesterday.
|
||||
winston.query(options, function (err, results) {
|
||||
//callback with results
|
||||
});
|
||||
// Find items logged between today and yesterday.
|
||||
winston.query(options, function (err, results) {
|
||||
//callback with results
|
||||
});
|
||||
|
||||
```
|
||||
|
||||
|
||||
@ -14,19 +14,20 @@ From the blog Ben Nadel, ranked 5 for the keywords “Node.JS error object”
|
||||
|
||||
```javascript
|
||||
//throwing an Error from typical function, whether sync or async
|
||||
if(!productToAdd)
|
||||
throw new Error("How can I add new product when no value provided?");
|
||||
if(!productToAdd)
|
||||
throw new Error("How can I add new product when no value provided?");
|
||||
|
||||
//'throwing' an Error from EventEmitter
|
||||
const myEmitter = new MyEmitter();
|
||||
myEmitter.emit('error', new Error('whoops!'));
|
||||
|
||||
//'throwing' an Error from a Promise
|
||||
return new promise(function (resolve, reject) {
|
||||
Return DAL.getProduct(productToAdd.id).then((existingProduct) =>{
|
||||
return new Promise(function (resolve, reject) {
|
||||
Return DAL.getProduct(productToAdd.id).then((existingProduct) =>{
|
||||
if(existingProduct != null)
|
||||
reject(new Error("Why fooling us and trying to add an existing product?"));
|
||||
|
||||
reject(new Error("Why fooling us and trying to add an existing product?"));
|
||||
})
|
||||
});
|
||||
```
|
||||
|
||||
### Code example – Anti Pattern
|
||||
@ -55,7 +56,7 @@ module.exports.appError = appError;
|
||||
|
||||
//client throwing an exception
|
||||
if(user == null)
|
||||
throw new appError(commonErrors.resourceNotFound, commonHTTPErrors.notFound, "further explanation", true)
|
||||
throw new appError(commonErrors.resourceNotFound, commonHTTPErrors.notFound, "further explanation", true)
|
||||
```
|
||||
|
||||
|
||||
|
||||
@ -17,9 +17,9 @@ This approach:
|
||||
|
||||
```javascript
|
||||
//Typical mistake 1: saving uploaded files locally in a server
|
||||
var multer = require('multer') //express middleware for fetching uploads
|
||||
var upload = multer({ dest: 'uploads/' })
|
||||
app.post('/photos/upload', upload.array('photos', 12), function (req, res, next) {})
|
||||
var multer = require('multer'); //express middleware for fetching uploads
|
||||
var upload = multer({ dest: 'uploads/' });
|
||||
app.post('/photos/upload', upload.array('photos', 12), function (req, res, next) {});
|
||||
//Typical mistake 2: storing authentication sessions (passport) in a local file or memory
|
||||
var FileStore = require('session-file-store')(session);
|
||||
app.use(session({
|
||||
@ -27,7 +27,7 @@ app.use(session({
|
||||
secret: 'keyboard cat'
|
||||
}));
|
||||
//Typical mistake3: storing information on the global object
|
||||
Global.someCacheLike.result = {somedata}
|
||||
Global.someCacheLike.result = { somedata };
|
||||
```
|
||||
|
||||
<br/><br/>
|
||||
|
||||
@ -16,10 +16,10 @@ A maintenance endpoint is a plain secured HTTP API that is part of the app code
|
||||
var heapdump = require('heapdump');
|
||||
|
||||
router.get('/ops/headump', (req, res, next) => {
|
||||
logger.info(`About to generate headump`);
|
||||
heapdump.writeSnapshot(function (err, filename) {
|
||||
logger.info('About to generate headump');
|
||||
heapdump.writeSnapshot((err, filename) => {
|
||||
console.log('headump file is ready to be sent to the caller', filename);
|
||||
fs.readFile(filename, "utf-8", function (err, data) {
|
||||
fs.readFile(filename, "utf-8", (err, data) => {
|
||||
res.end(data);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user