Skip to content

Commit db746e3

Browse files
digitalsadhudigitalsadhu
authored
Merge pull request #130 from digitalsadhu/remove_error_generalisation
fix(http status codes): Remove status code generalisation
2 parents adfe62e + 902f31e commit db746e3

File tree

8 files changed

+37
-65
lines changed

8 files changed

+37
-65
lines changed

‎lib/errors.js‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@ function JSONAPIErrorHandler (err, req, res, next) {
4747
)
4848
})
4949
} else if (err.message) {
50-
debug('Handling error generally. Eg. 404, 500.')
51-
// this block is for handling other non validation related errors. 404s, 500s etc.
52-
5350
// convert specific errors below to validation errors.
5451
// These errors are from checks on the incoming payload to ensure it is
5552
// JSON API compliant. If we switch to being able to use the Accept header
@@ -83,37 +80,13 @@ function JSONAPIErrorHandler (err, req, res, next) {
8380
errors.push(buildErrorResponse(statusCodes.INTERNAL_SERVER_ERROR, 'Internal Server error', 'GENERAL_SERVER_ERROR'))
8481
}
8582

86-
// generalise the error code. More specific error codes are kept
87-
// with each error object
88-
debug('Generalising error status code. Eg. 404 -> 400 for the response')
89-
statusCode = generalizeStatusCode(statusCode)
90-
9183
// send the errors and close out the response.
9284
debug('Sending error response')
9385
debug('Response Code:', statusCode)
9486
debug('Response Object:', {errors: errors})
9587
res.status(statusCode).send({errors: errors}).end()
9688
}
9789

98-
/**
99-
* Takes a more specific error code and generalises it to 400 or 500.
100-
* If a generalisation cannot occur, the original code will be returned.
101-
* @private
102-
* @memberOf {Errors}
103-
* @param {number} statusCode
104-
* @return {number}
105-
*/
106-
function generalizeStatusCode (statusCode) {
107-
switch (String(statusCode)[0]) {
108-
case '4':
109-
return statusCodes.BAD_REQUEST
110-
case '5':
111-
return statusCodes.INTERNAL_SERVER_ERROR
112-
}
113-
114-
return statusCode
115-
}
116-
11790
/**
11891
* Builds an error object for sending to the user.
11992
* @private

‎test/belongsTo.test.js‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -434,39 +434,41 @@ describe('loopback json api belongsTo relationships', function () {
434434
beforeEach(function (done) {
435435
request(app).post('/posts/')
436436
.send({
437-
'data': {
438-
'type': 'posts',
439-
'attributes': { 'title': 'Post 1', 'content': 'This is my first post'}
440-
}})
437+
data: {
438+
type: 'posts',
439+
attributes: { title: 'Post 1', content: 'This is my first post' }
440+
}
441+
})
441442
.set('Accept', 'application/vnd.api+json')
442443
.set('Content-Type', 'application/json')
443444
.end(function (err) {
444445
expect(err).to.equal(null)
445446
request(app).post('/posts')
446447
.send({
447-
'data': {
448-
'type': 'posts',
449-
'attributes': {'title': 'Post 2', 'content': 'This is my second post'}
450-
}})
448+
data: {
449+
type: 'posts',
450+
attributes: {title: 'Post 2', content: 'This is my second post'}
451+
}
452+
})
451453
.set('Accept', 'application/vnd.api+json')
452454
.set('Content-Type', 'application/json')
453455
.end(function (err) {
454456
expect(err).to.equal(null)
455457
request(app).post('/comments')
456458
.send({'data': {
457-
'type': 'comments',
458-
'attributes': {'title': 'First comment', 'comment': 'Comment 1 text'},
459-
'relationships': {'post': {'data': {'type': 'posts', 'id': '1'}}}
459+
'type': 'comments',
460+
'attributes': {'title': 'First comment', 'comment': 'Comment 1 text'},
461+
'relationships': {'post': {'data': {'type': 'posts', 'id': '1'}}}
460462
}})
461463
.set('Accept', 'application/vnd.api+json')
462464
.set('Content-Type', 'application/json')
463465
.end(function (err) {
464466
expect(err).to.equal(null)
465467
request(app).post('/comments')
466468
.send({'data': {
467-
'type': 'comments:',
468-
'attributes': {'title': 'Second comment', 'comment': 'Comment 2 text'},
469-
'relationships': {'post': {'data': {'type': ' posts', 'id': '2'}}}
469+
'type': 'comments:',
470+
'attributes': {'title': 'Second comment', 'comment': 'Comment 2 text'},
471+
'relationships': {'post': {'data': {'type': ' posts', 'id': '2'}}}
470472
}})
471473
.set('Accept', 'application/vnd.api+json')
472474
.set('Content-Type', 'application/json')

‎test/config.test.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ describe('Dont override config.js ', function () {
5959
})
6060
.set('Accept', 'application/vnd.api+json')
6161
.set('Content-Type', 'application/json')
62-
.expect(400)
62+
.expect(413)
6363
.end(done)
6464
})
65-
6665
})

‎test/errors.test.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@ describe('loopback json api errors', function () {
6060
})
6161

6262
describe('status codes', function () {
63-
it('GET /models/100 should return a generalized 400 not found error', function (done) {
63+
it('GET /models/100 should return a 404 not found error', function (done) {
6464
request(app).get('/posts/100')
65-
.expect(400)
65+
.expect(404)
6666
.end(done)
6767
})
6868

69-
it('GET /models/100 should return a generalized 400 not found error', function (done) {
69+
it('GET /models/100 should return a 404 not found error', function (done) {
7070
request(app).get('/posts/100')
7171
.end(function (err, res) {
7272
expect(err).to.equal(null)
@@ -83,17 +83,17 @@ describe('loopback json api errors', function () {
8383
})
8484
})
8585

86-
it('POST /models should return a general 400 status code if type key is not present', function (done) {
86+
it('POST /models should return a 422 status code if type key is not present', function (done) {
8787
request(app).post('/posts')
88-
.send({ data: { attributes: { title: 'my post', content: 'my post content' }}})
89-
.expect(400)
88+
.send({ data: { attributes: { title: 'my post', content: 'my post content' } } })
89+
.expect(422)
9090
.set('Content-Type', 'application/json')
9191
.end(done)
9292
})
9393

9494
it('POST /models should return a more specific 422 status code on the error object if type key is not present', function (done) {
9595
request(app).post('/posts')
96-
.send({ data: { attributes: { title: 'my post', content: 'my post content' }}})
96+
.send({ data: { attributes: { title: 'my post', content: 'my post content' } } })
9797
.set('Content-Type', 'application/json')
9898
.end(function (err, res) {
9999
expect(err).to.equal(null)
@@ -110,12 +110,12 @@ describe('loopback json api errors', function () {
110110
})
111111
})
112112

113-
it('POST /models should return an 400 error if model title is not present', function (done) {
113+
it('POST /models should return an 422 error if model title is not present', function (done) {
114114
Post.validatesPresenceOf('title')
115115

116116
request(app).post('/posts')
117117
.send({ data: { type: 'posts' } })
118-
.expect(400)
118+
.expect(422)
119119
.set('Content-Type', 'application/json')
120120
.end(done)
121121
})

‎test/find.test.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('loopback json api component find methods', function () {
3030

3131
it('GET /models/:id should have the JSON API Content-Type header set on individual resource responses', function (done) {
3232
request(app).get('/posts/1')
33-
.expect(400)
33+
.expect(404)
3434
.expect('Content-Type', 'application/vnd.api+json; charset=utf-8')
3535
.end(done)
3636
})
@@ -156,9 +156,9 @@ describe('loopback json api component find methods', function () {
156156
})
157157
})
158158

159-
it('GET model/:id should return a general 400 when there are no results', function (done) {
159+
it('GET model/:id should return a general 404 when there are no results', function (done) {
160160
request(app).get('/posts/1')
161-
.expect(400)
161+
.expect(404)
162162
.end(done)
163163
})
164164
})
@@ -207,9 +207,9 @@ describe('loopback json api component find methods', function () {
207207
})
208208

209209
describe('Errors', function () {
210-
it('GET /models/doesnt/exist should return a general 400 error', function (done) {
210+
it('GET /models/doesnt/exist should return a general 404 error', function (done) {
211211
request(app).get('/posts/doesnt/exist')
212-
.expect(400)
212+
.expect(404)
213213
.end(done)
214214
})
215215
})

‎test/hasMany.test.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ describe('loopback json api hasMany relationships', function () {
312312
.end(done)
313313
})
314314
})
315-
316315
})
317316

318317
describe('Nested relationships', function () {

‎test/hasOne.test.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,5 @@ describe('loopback json api hasOne relationships', function () {
283283
.end(done)
284284
})
285285
})
286-
287286
})
288287
})

‎test/update.test.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('loopback json api component update method', function () {
7070
.end(done)
7171
})
7272

73-
it('PATCH /models/:id should return 400 when attempting to edit non existing resource', function (done) {
73+
it('PATCH /models/:id should return 404 when attempting to edit non existing resource', function (done) {
7474
request(app).patch('/posts/10')
7575
.send({
7676
data: {
@@ -82,7 +82,7 @@ describe('loopback json api component update method', function () {
8282
}
8383
}
8484
})
85-
.expect(400)
85+
.expect(404)
8686
.end(done)
8787
})
8888
})
@@ -211,7 +211,7 @@ describe('loopback json api component update method', function () {
211211
})
212212
})
213213

214-
it('PATCH /models/:id should return an 400 error if `type` key is not present and include an errors array', function (done) {
214+
it('PATCH /models/:id should return an 422 error if `type` key is not present and include an errors array', function (done) {
215215
request(app).patch('/posts/1')
216216
.send({
217217
data: {
@@ -222,7 +222,7 @@ describe('loopback json api component update method', function () {
222222
}
223223
}
224224
})
225-
.expect(400)
225+
.expect(422)
226226
.set('Content-Type', 'application/json')
227227
.end(function (err, res) {
228228
expect(err).to.equal(null)
@@ -236,7 +236,7 @@ describe('loopback json api component update method', function () {
236236
})
237237
})
238238

239-
it('PATCH /models/:id should return an 400 error if `id` key is not present and include an errors array', function (done) {
239+
it('PATCH /models/:id should return an 422 error if `id` key is not present and include an errors array', function (done) {
240240
request(app).patch('/posts/1')
241241
.send({
242242
data: {
@@ -247,7 +247,7 @@ describe('loopback json api component update method', function () {
247247
}
248248
}
249249
})
250-
.expect(400)
250+
.expect(422)
251251
.set('Content-Type', 'application/json')
252252
.end(function (err, res) {
253253
expect(err).to.equal(null)

0 commit comments

Comments
 (0)