ComputerSurge/shields
3
0
Fork 0
Code Pull Requests 1 Activity

Provide a descriptive message when json can’t be parsed (#1796)

Browse Source 
* Test handling of invalid JSON on BaseJsonService
* Remove invalidJSON tests from new-style services
This commit is contained in:
Paul Melnikow
2018-07-23 19:35:42 -05:00
committed by GitHub
parent f5fe85cda2
commit 6e09a93cf0
5 changed files with 46 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
lib/error-helper.js
View File

@@ -39,9 +39,12 @@ async function asJson({ buffer, res }) {
try {
return JSON.parse(buffer);
} catch (err) {
throw new InvalidResponse({ underlyingError: err });
throw new InvalidResponse({
prettyMessage: 'unparseable json response',
underlyingError: err,
});
}
};
}
module.exports = {
checkErrorResponse,

41
services/base-json.spec.js Normal file
View File

@@ -0,0 +1,41 @@
'use strict';
const { expect } = require('chai');
const { BaseJsonService } = require('./base');
const { invalidJSON } = require('./response-fixtures');
class DummyJsonService extends BaseJsonService {
static get category() {
return 'cat';
}
static get url() {
return {
base: 'foo',
};
}
async handle() {
const { value } = await this._requestJson();
return { message: value };
}
}
describe('BaseJsonService', () => {
it('handles unparseable json responses', async function() {
const sendAndCacheRequest = async () => ({
buffer: invalidJSON,
res: { statusCode: 200 },
});
const serviceInstance = new DummyJsonService(
{ sendAndCacheRequest },
{ handleInternalErrors: false }
);
const serviceData = await serviceInstance.invokeHandler({}, {});
expect(serviceData).to.deep.equal({
color: 'lightgray',
message: 'unparseable json response',
});
});
});

9
services/cdnjs/cdnjs.tester.js
View File

@@ -3,7 +3,6 @@
const Joi = require('joi');
const ServiceTester = require('../service-tester');
const { isVPlusTripleDottedVersion } = require('../test-validators');
const { invalidJSON } = require('../response-fixtures');
const t = new ServiceTester({ id: 'cdnjs', title: 'CDNJs' });
module.exports = t;
@@ -25,14 +24,6 @@ t.create('cdnjs (connection error)')
.networkOff()
.expectJSON({name: 'cdnjs', value: 'inaccessible'});
t.create('cdnjs (unexpected response)')
.get('/v/jquery.json')
.intercept(nock => nock('https://api.cdnjs.com')
.get('/libraries/jquery?fields=version')
.reply(invalidJSON)
)
.expectJSON({name: 'cdnjs', value: 'invalid'});
t.create('cdnjs (error response)')
.get('/v/jquery.json')
.intercept(nock => nock('https://api.cdnjs.com')

9
services/clojars/clojars.tester.js
View File

@@ -2,7 +2,6 @@
const Joi = require('joi');
const ServiceTester = require('../service-tester');
const { invalidJSON } = require('../response-fixtures');
const t = new ServiceTester({ id: 'clojars', title: 'clojars' });
module.exports = t;
@@ -24,14 +23,6 @@ t.create('clojars (connection error)')
.networkOff()
.expectJSON({name: 'clojars', value: 'inaccessible'});
t.create('clojars (unexpected response)')
.get('/v/prismic.json')
.intercept(nock => nock('https://clojars.org')
.get('/prismic/latest-version.json')
.reply(invalidJSON)
)
.expectJSON({name: 'clojars', value: 'invalid'});
t.create('clojars (error response)')
.get('/v/prismic.json')
.intercept(nock => nock('https://clojars.org')

49
services/gem/gem.tester.js
View File

@@ -7,7 +7,6 @@ const {
isVPlusDottedVersionAtLeastOne,
isMetric
} = require('../test-validators');
const { invalidJSON } = require('../response-fixtures');
const isOrdinalNumber = Joi.string().regex(/^[1-9][0-9]+(ᵗʰ|ˢᵗ|ⁿᵈ|ʳᵈ)$/);
const isOrdinalNumberDaily = Joi.string().regex(/^[1-9][0-9]+(ᵗʰ|ˢᵗ|ⁿᵈ|ʳᵈ) daily$/);
@@ -33,14 +32,6 @@ t.create('version (connection error)')
.networkOff()
.expectJSON({name: 'gem', value: 'inaccessible'});
t.create('version (unexpected response)')
.get('/v/formatador.json')
.intercept(nock => nock('https://rubygems.org')
.get('/api/v1/gems/formatador.json')
.reply(invalidJSON)
)
.expectJSON({name: 'gem', value: 'invalid'});
// downloads endpoints
@@ -61,14 +52,6 @@ t.create('total downloads (connection error)')
.networkOff()
.expectJSON({name: 'downloads', value: 'inaccessible'});
t.create('total downloads (unexpected response)')
.get('/dt/rails.json')
.intercept(nock => nock('https://rubygems.org')
.get('/api/v1/gems/rails.json')
.reply(invalidJSON)
)
.expectJSON({name: 'downloads', value: 'invalid'});
// version downloads
t.create('version downloads (valid, stable version)')
@@ -102,14 +85,6 @@ t.create('version downloads (connection error)')
.networkOff()
.expectJSON({name: 'downloads', value: 'inaccessible'});
t.create('version downloads (unexpected response)')
.get('/dv/rails/4.1.0.json')
.intercept(nock => nock('https://rubygems.org')
.get('/api/v1/versions/rails.json')
.reply(invalidJSON)
)
.expectJSON({name: 'downloads', value: 'invalid'});
// latest version downloads
t.create('latest version downloads (valid)')
@@ -128,14 +103,6 @@ t.create('latest version downloads (connection error)')
.networkOff()
.expectJSON({name: 'downloads', value: 'inaccessible'});
t.create('latest version downloads (unexpected response)')
.get('/dtv/rails.json')
.intercept(nock => nock('https://rubygems.org')
.get('/api/v1/gems/rails.json')
.reply(invalidJSON)
)
.expectJSON({name: 'downloads', value: 'invalid'});
// users endpoint
@@ -155,14 +122,6 @@ t.create('users (connection error)')
.networkOff()
.expectJSON({name: 'gems', value: 'inaccessible'});
t.create('users (unexpected response)')
.get('/u/raphink.json')
.intercept(nock => nock('https://rubygems.org')
.get('/api/v1/owners/raphink/gems.json')
.reply(invalidJSON)
)
.expectJSON({name: 'gems', value: 'invalid'});
// rank endpoint
@@ -188,11 +147,3 @@ t.create('rank (connection error)')
.get('/rt/rspec-puppet-facts.json')
.networkOff()
.expectJSON({name: 'rank', value: 'inaccessible'});
t.create('rank (unexpected response)')
.get('/rt/rspec-puppet-facts.json')
.intercept(nock => nock('http://bestgems.org')
.get('/api/v1/gems/rspec-puppet-facts/total_ranking.json')
.reply(invalidJSON)
)
.expectJSON({name: 'rank', value: 'invalid'});