Skip to content

Commit 53ef68d

Browse files
jmdobryjmdobry
committed
Fixes GoogleCloudPlatform#38 so errors will be logged properly. Cleaned up some code style and added some comments.
1 parent df3a7b6 commit 53ef68d

File tree

18 files changed

+957
-593
lines changed

18 files changed

+957
-593
lines changed

‎2-structured-data/app.js

+10-6
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,28 @@ app.set('views', path.join(__dirname, 'views'));
2424
app.set('view engine', 'jade');
2525
app.set('trust proxy', true);
2626

27-
2827
// Books
2928
var model = require('./books/model-' + config.dataBackend)(config);
3029
app.use('/books', require('./books/crud')(model));
3130
app.use('/api/books', require('./books/api')(model));
3231

33-
3432
// Redirect root to /books
35-
app.get('/', function(req, res) {
33+
app.get('/', function (req, res) {
3634
res.redirect('/books');
3735
});
3836

37+
// Basic 404 handler
38+
app.use(function (req, res) {
39+
res.status(404).send('Not Found');
40+
});
3941

4042
// Basic error handler
41-
app.use(function(err, req, res, next) {
43+
app.use(function (err, req, res, next) {
4244
/* jshint unused:false */
43-
console.error(err.stack);
44-
res.status(500).send('Something broke!');
45+
console.error(err);
46+
// If our routes specified a specific response, then send that. Otherwise,
47+
// send a generic message so as not to leak anything.
48+
res.status(500).send(err.response || 'Something broke!');
4549
});
4650

4751
if (module === require.main) {

‎2-structured-data/books/api.js

+59-37
Original file line numberDiff line numberDiff line change
@@ -16,66 +16,88 @@
1616
var express = require('express');
1717
var bodyParser = require('body-parser');
1818

19-
20-
module.exports = function(model) {
19+
module.exports = function (model) {
2120

2221
var router = express.Router();
2322

23+
// Automatically parse request body as JSON
2424
router.use(bodyParser.json());
2525

26-
27-
function handleRpcError(err, res) {
28-
if (err.code === 404) { return res.status(404); }
29-
res.status(500).json({
30-
message: err.message,
31-
internalCode: err.code
32-
});
33-
}
34-
35-
36-
router.get('/', function list(req, res) {
37-
model.list(10, req.query.pageToken,
38-
function(err, entities, cursor) {
39-
if (err) { return handleRpcError(err, res); }
40-
res.json({
41-
items: entities,
42-
nextPageToken: cursor
43-
});
26+
/**
27+
* GET /api/books
28+
*
29+
* Retrieve a page of books (up to ten at a time).
30+
*/
31+
router.get('/', function list(req, res, next) {
32+
model.list(10, req.query.pageToken, function (err, entities, cursor) {
33+
if (err) { return next(err); }
34+
res.json({
35+
items: entities,
36+
nextPageToken: cursor
4437
});
38+
});
4539
});
4640

47-
48-
router.post('/', function insert(req, res) {
49-
model.create(req.body, function(err, entity) {
50-
if (err) { return handleRpcError(err, res); }
41+
/**
42+
* POST /api/books
43+
*
44+
* Create a new book.
45+
*/
46+
router.post('/', function insert(req, res, next) {
47+
model.create(req.body, function (err, entity) {
48+
if (err) { return next(err); }
5149
res.json(entity);
5250
});
5351
});
5452

55-
56-
router.get('/:book(\\d+)', function get(req, res) {
57-
model.read(req.params.book, function(err, entity) {
58-
if (err) { return handleRpcError(err, res); }
53+
/**
54+
* GET /api/books/:id
55+
*
56+
* Retrieve a book.
57+
*/
58+
router.get('/:book(\\d+)', function get(req, res, next) {
59+
model.read(req.params.book, function (err, entity) {
60+
if (err) { return next(err); }
5961
res.json(entity);
6062
});
6163
});
6264

63-
64-
router.put('/:book(\\d+)', function update(req, res) {
65-
model.update(req.params.book, req.body, function(err, entity) {
66-
if (err) { return handleRpcError(err, res); }
65+
/**
66+
* PUT /api/books/:id
67+
*
68+
* Update a book.
69+
*/
70+
router.put('/:book(\\d+)', function update(req, res, next) {
71+
model.update(req.params.book, req.body, function (err, entity) {
72+
if (err) { return next(err); }
6773
res.json(entity);
6874
});
6975
});
7076

71-
72-
router.delete('/:book(\\d+)', function _delete(req, res) {
73-
model.delete(req.params.book, function(err) {
74-
if (err) { return handleRpcError(err, res); }
77+
/**
78+
* DELETE /api/books/:id
79+
*
80+
* Delete a book.
81+
*/
82+
router.delete('/:book(\\d+)', function _delete(req, res, next) {
83+
model.delete(req.params.book, function (err) {
84+
if (err) { return next(err); }
7585
res.status(200).send('OK');
7686
});
7787
});
7888

79-
return router;
89+
/**
90+
* Errors on "/api/books/*" routes.
91+
*/
92+
router.use(function handleRpcError(err, req, res, next) {
93+
// Format error and forward to generic error handler for logging and
94+
// responding to the request
95+
err.response = {
96+
message: err.message,
97+
internalCode: err.code
98+
};
99+
next(err);
100+
});
80101

102+
return router;
81103
};

‎2-structured-data/books/crud.js

+78-49
Original file line numberDiff line numberDiff line change
@@ -16,100 +16,129 @@
1616
var express = require('express');
1717
var bodyParser = require('body-parser');
1818

19-
20-
module.exports = function(model) {
19+
module.exports = function (model) {
2120

2221
var router = express.Router();
2322

24-
router.use(bodyParser.urlencoded({extended: false}));
25-
26-
27-
function handleRpcError(err, res) {
28-
res.status(err.code || 500).send(err.message);
29-
}
23+
// Automatically parse request body as form data
24+
router.use(bodyParser.urlencoded({ extended: false }));
3025

31-
32-
router.use(function(req, res, next){
26+
// Set Content-Type for all responses for these routes
27+
router.use(function (req, res, next){
3328
res.set('Content-Type', 'text/html');
3429
next();
3530
});
3631

37-
38-
router.get('/', function list(req, res) {
39-
model.list(10, req.query.pageToken,
40-
function(err, entities, cursor) {
41-
if (err) { return handleRpcError(err, res); }
42-
res.render('books/list.jade', {
43-
books: entities,
44-
nextPageToken: cursor
45-
});
46-
}
47-
);
32+
/**
33+
* GET /books/add
34+
*
35+
* Display a page of books (up to ten at a time).
36+
*/
37+
router.get('/', function list(req, res, next) {
38+
model.list(10, req.query.pageToken, function (err, entities, cursor) {
39+
if (err) { return next(err); }
40+
res.render('books/list.jade', {
41+
books: entities,
42+
nextPageToken: cursor
43+
});
44+
});
4845
});
4946

50-
51-
// [START add_get]
47+
/**
48+
* GET /books/add
49+
*
50+
* Display a form for creating a book.
51+
*/
52+
// [START add_get]
5253
router.get('/add', function addForm(req, res) {
5354
res.render('books/form.jade', {
5455
book: {},
5556
action: 'Add'
5657
});
5758
});
58-
// [END add_get]
59-
60-
61-
// [START add_post]
62-
router.post('/add', function insert(req, res) {
59+
// [END add_get]
60+
61+
/**
62+
* POST /books/add
63+
*
64+
* Create a book.
65+
*/
66+
// [START add_post]
67+
router.post('/add', function insert(req, res, next) {
6368
var data = req.body;
6469

6570
// Save the data to the database.
66-
model.create(data, function(err, savedData) {
67-
if (err) { return handleRpcError(err, res); }
71+
model.create(data, function (err, savedData) {
72+
if (err) { return next(err); }
6873
res.redirect(req.baseUrl + '/' + savedData.id);
6974
});
7075
});
71-
// [END add_post]
72-
73-
74-
router.get('/:book/edit', function editForm(req, res) {
75-
model.read(req.params.book, function(err, entity) {
76-
if (err) { return handleRpcError(err, res); }
76+
// [END add_post]
77+
78+
/**
79+
* GET /books/:id/edit
80+
*
81+
* Display a book for editing.
82+
*/
83+
router.get('/:book/edit', function editForm(req, res, next) {
84+
model.read(req.params.book, function (err, entity) {
85+
if (err) { return next(err); }
7786
res.render('books/form.jade', {
7887
book: entity,
7988
action: 'Edit'
8089
});
8190
});
8291
});
8392

84-
85-
router.post('/:book/edit', function update(req, res) {
93+
/**
94+
* POST /books/:id/edit
95+
*
96+
* Update a book.
97+
*/
98+
router.post('/:book/edit', function update(req, res, next) {
8699
var data = req.body;
87100

88-
model.update(req.params.book, data, function(err, savedData) {
89-
if (err) { return handleRpcError(err, res); }
101+
model.update(req.params.book, data, function (err, savedData) {
102+
if (err) { return next(err); }
90103
res.redirect(req.baseUrl + '/' + savedData.id);
91104
});
92105
});
93106

94-
95-
router.get('/:book', function get(req, res) {
96-
model.read(req.params.book, function(err, entity) {
97-
if (err) { return handleRpcError(err, res); }
107+
/**
108+
* GET /books/:id
109+
*
110+
* Display a book.
111+
*/
112+
router.get('/:book', function get(req, res, next) {
113+
model.read(req.params.book, function (err, entity) {
114+
if (err) { return next(err); }
98115
res.render('books/view.jade', {
99116
book: entity
100117
});
101118
});
102119
});
103120

104-
105-
router.get('/:book/delete', function _delete(req, res) {
106-
model.delete(req.params.book, function(err) {
107-
if (err) { return handleRpcError(err, res); }
121+
/**
122+
* GET /books/:id/delete
123+
*
124+
* Delete a book.
125+
*/
126+
router.get('/:book/delete', function _delete(req, res, next) {
127+
model.delete(req.params.book, function (err) {
128+
if (err) { return next(err); }
108129
res.redirect(req.baseUrl);
109130
});
110131
});
111132

133+
/**
134+
* Errors on "/books/*" routes.
135+
*/
136+
router.use(function handleRpcError(err, req, res, next) {
137+
// Format error and forward to generic error handler for logging and
138+
// responding to the request
139+
err.response = err.message;
140+
next(err);
141+
});
112142

113143
return router;
114-
115144
};

‎3-binary-data/app.js

+10-7
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ app.set('views', path.join(__dirname, 'views'));
2424
app.set('view engine', 'jade');
2525
app.set('trust proxy', true);
2626

27-
2827
// Setup modules and dependencies
2928
var images = require('./lib/images')(config.gcloud, config.cloudStorageBucket);
3029
var model = require('./books/model-' + config.dataBackend)(config);
@@ -33,21 +32,25 @@ var model = require('./books/model-' + config.dataBackend)(config);
3332
app.use('/books', require('./books/crud')(model, images));
3433
app.use('/api/books', require('./books/api')(model));
3534

36-
3735
// Redirect root to /books
38-
app.get('/', function(req, res) {
36+
app.get('/', function (req, res) {
3937
res.redirect('/books');
4038
});
4139

40+
// Basic 404 handler
41+
app.use(function (req, res) {
42+
res.status(404).send('Not Found');
43+
});
4244

4345
// Basic error handler
44-
app.use(function(err, req, res, next) {
46+
app.use(function (err, req, res, next) {
4547
/* jshint unused:false */
46-
console.error(err.stack);
47-
res.status(500).send('Something broke!');
48+
console.error(err);
49+
// If our routes specified a specific response, then send that. Otherwise,
50+
// send a generic message so as not to leak anything.
51+
res.status(500).send(err.response || 'Something broke!');
4852
});
4953

50-
5154
if (module === require.main) {
5255
// Start the server
5356
var server = app.listen(config.port, function () {

0 commit comments

Comments
 (0)