Skip to content

Commit

Permalink
Merge pull request goldbergyoni#116 from AllenFang/master
Browse files Browse the repository at this point in the history
prettify and unify the syntax
  • Loading branch information
BrunoScheufler authored Dec 27, 2017
2 parents b7c9fe4 + 89f77b0 commit 2c0bb75
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 68 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)



Expand Down
19 changes: 9 additions & 10 deletions sections/errorhandling/catchunhandledpromiserejection.md
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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))
Expand All @@ -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’);
});
```

Expand Down
8 changes: 4 additions & 4 deletions sections/errorhandling/centralizedhandling.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand All @@ -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);
Expand Down
19 changes: 9 additions & 10 deletions sections/errorhandling/shuttingtheprocess.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

```
Expand Down
33 changes: 19 additions & 14 deletions sections/errorhandling/testingerrorflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

```
Expand All @@ -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();
});
});

```
34 changes: 19 additions & 15 deletions sections/errorhandling/usematurelogger.md
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand All @@ -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']
};


// Find items logged between today and yesterday.
winston.query(options, function (err, results) {
//callback with results
});
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
});

```

Expand Down
15 changes: 8 additions & 7 deletions sections/errorhandling/useonlythebuiltinerror.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
```


Expand Down
8 changes: 4 additions & 4 deletions sections/production/bestateless.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ 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({
store: new FileStore(options),
secret: 'keyboard cat'
}));
//Typical mistake3: storing information on the global object
Global.someCacheLike.result = {somedata}
Global.someCacheLike.result = { somedata };
```

<br/><br/>
Expand Down
6 changes: 3 additions & 3 deletions sections/production/createmaintenanceendpoint.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down

0 comments on commit 2c0bb75

Please sign in to comment.