From 45c29a7b5152858ee4dcd837a4a151b9f7490784 Mon Sep 17 00:00:00 2001 From: AllenFang Date: Tue, 26 Dec 2017 22:55:37 +0800 Subject: [PATCH 1/3] prettify and unify the syntax --- .../catchunhandledpromiserejection.md | 19 +++++------ sections/errorhandling/centralizedhandling.md | 8 ++--- sections/errorhandling/shuttingtheprocess.md | 19 +++++------ sections/errorhandling/testingerrorflows.md | 33 +++++++++++-------- sections/errorhandling/usematurelogger.md | 30 +++++++++-------- .../errorhandling/useonlythebuiltinerror.md | 15 +++++---- sections/production/bestateless.md | 8 ++--- .../production/createmaintenanceendpoint.md | 6 ++-- 8 files changed, 73 insertions(+), 65 deletions(-) diff --git a/sections/errorhandling/catchunhandledpromiserejection.md b/sections/errorhandling/catchunhandledpromiserejection.md index e17625ac..d7a54e30 100644 --- a/sections/errorhandling/catchunhandledpromiserejection.md +++ b/sections/errorhandling/catchunhandledpromiserejection.md @@ -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’); }); ``` diff --git a/sections/errorhandling/centralizedhandling.md b/sections/errorhandling/centralizedhandling.md index 155f7d3d..fbe82f6d 100644 --- a/sections/errorhandling/centralizedhandling.md +++ b/sections/errorhandling/centralizedhandling.md @@ -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); diff --git a/sections/errorhandling/shuttingtheprocess.md b/sections/errorhandling/shuttingtheprocess.md index 539d716c..5e36440f 100644 --- a/sections/errorhandling/shuttingtheprocess.md +++ b/sections/errorhandling/shuttingtheprocess.md @@ -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; +} ``` diff --git a/sections/errorhandling/testingerrorflows.md b/sections/errorhandling/testingerrorflows.md index 0684cab7..a85d537d 100644 --- a/sections/errorhandling/testingerrorflows.md +++ b/sections/errorhandling/testingerrorflows.md @@ -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(); + }); +}); ``` \ No newline at end of file diff --git a/sections/errorhandling/usematurelogger.md b/sections/errorhandling/usematurelogger.md index 098d992b..9080a980 100644 --- a/sections/errorhandling/usematurelogger.md +++ b/sections/errorhandling/usematurelogger.md @@ -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 +}); ``` diff --git a/sections/errorhandling/useonlythebuiltinerror.md b/sections/errorhandling/useonlythebuiltinerror.md index 60397bd7..5234c022 100644 --- a/sections/errorhandling/useonlythebuiltinerror.md +++ b/sections/errorhandling/useonlythebuiltinerror.md @@ -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) ``` diff --git a/sections/production/bestateless.md b/sections/production/bestateless.md index 86f8c2c4..4cfd9805 100644 --- a/sections/production/bestateless.md +++ b/sections/production/bestateless.md @@ -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 }; ```

diff --git a/sections/production/createmaintenanceendpoint.md b/sections/production/createmaintenanceendpoint.md index fc0601fa..2c03d87b 100644 --- a/sections/production/createmaintenanceendpoint.md +++ b/sections/production/createmaintenanceendpoint.md @@ -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); }); }); From 1be21cdf7934ba38cbac701a28e16a759beaf590 Mon Sep 17 00:00:00 2001 From: AllenFang Date: Wed, 27 Dec 2017 11:26:39 +0800 Subject: [PATCH 2/3] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d6c4ed39..b8356465 100644 --- a/README.md +++ b/README.md @@ -723,6 +723,7 @@ This repository is being kept up to date thanks to the help from the community. 🌻 [Xavier Ho](https://github.com/spaxe), 🌻 [Aaron Arney](https://github.com/ocularrhythm), 🌻 [Jan Charles Maghirang Adona](https://github.com/septa97) +🌻 [Allen Fang](https://github.com/AllenFang) From 89f77b043a46979abcb4e86313c5a6b2c033cc13 Mon Sep 17 00:00:00 2001 From: Bruno Scheufler <4772980+BrunoScheufler@users.noreply.github.com> Date: Wed, 27 Dec 2017 12:49:18 +0100 Subject: [PATCH 3/3] Fixed Thank You Notes section layout --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b8356465..abfbad98 100644 --- a/README.md +++ b/README.md @@ -722,7 +722,7 @@ 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)