Skip to content

Commit 6d20222

Browse files
buunguyenbuunguyen
authored
Fix error handling (#1015)
1 parent 503c1f4 commit 6d20222

File tree

4 files changed

+87
-65
lines changed

4 files changed

+87
-65
lines changed

‎src/http.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export const self = {
1111

1212
// Handles fetch-like syntax and the case where there is only one object passed-in
1313
// (which will have the URL as a property). Also serilizes the response.
14-
export default function http(url, request={}) {
14+
export default function http(url, request = {}) {
1515
if (typeof url === 'object') {
1616
request = url
1717
url = request.url
@@ -30,13 +30,18 @@ export default function http(url, request={}) {
3030

3131
// for content-type=multipart\/form-data remove content-type from request before fetch
3232
// so that correct one with `boundary` is set
33-
let contentType = request.headers["content-type"] || request.headers["Content-Type"]
33+
const contentType = request.headers['content-type'] || request.headers['Content-Type']
3434
if (/multipart\/form-data/i.test(contentType)) {
3535
delete request.headers['content-type']
3636
delete request.headers['Content-Type']
3737
}
3838

3939
return fetch(request.url, request).then((res) => {
40+
if (!res.ok) {
41+
const error = new Error(res.statusText)
42+
error.statusCode = error.status = res.status
43+
throw error
44+
}
4045
return self.serializeRes(res, url, request).then((_res) => {
4146
if (request.responseInterceptor) {
4247
_res = request.responseInterceptor(_res) || _res
@@ -178,7 +183,7 @@ export function mergeInQueryOrForm(req = {}) {
178183
return isFile(form[key].value)
179184
})
180185

181-
let contentType = req.headers["content-type"] || req.headers["Content-Type"]
186+
const contentType = req.headers['content-type'] || req.headers['Content-Type']
182187

183188
if (hasFile || /multipart\/form-data/i.test(contentType)) {
184189
const FormData = require('isomorphic-form-data') // eslint-disable-line global-require

‎src/resolver.js‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ export function makeFetchJSON(http) {
1111
Accept: 'application/json'
1212
}
1313
})
14-
.then(res => res.body)
14+
.then((res) => {
15+
return res.body
16+
})
1517
}
1618
}
1719

@@ -30,29 +32,29 @@ export default function resolve({http, fetch, spec, url, baseDoc, mode, allowMet
3032
http = fetch || http || Http
3133

3234
if (!spec) {
33-
// We create a spec, that has a single $ref to the url
34-
// This is how we'll resolve it based on a URL only
35-
spec = {$ref: baseDoc}
36-
}
37-
else {
38-
// Store the spec into the url provided, to cache it
39-
plugins.refs.docCache[baseDoc] = spec
35+
return makeFetchJSON(http)(baseDoc).then(doResolve)
4036
}
4137

42-
// Build a json-fetcher ( ie: give it a URL and get json out )
43-
plugins.refs.fetchJSON = makeFetchJSON(http)
38+
return doResolve(spec)
4439

45-
const plugs = [plugins.refs]
40+
function doResolve(_spec) {
41+
plugins.refs.docCache[baseDoc] = _spec
4642

47-
if (mode !== 'strict') {
48-
plugs.push(plugins.allOf)
49-
}
43+
// Build a json-fetcher ( ie: give it a URL and get json out )
44+
plugins.refs.fetchJSON = makeFetchJSON(http)
5045

51-
// mapSpec is where the hard work happens, see https://github.com/swagger-api/specmap for more details
52-
return mapSpec({
53-
spec,
54-
context: {baseDoc},
55-
plugins: plugs,
56-
allowMetaPatches // allows adding .meta patches, which include adding `$$ref`s to the spec
57-
}).then(normalizeSwagger)
46+
const plugs = [plugins.refs]
47+
48+
if (mode !== 'strict') {
49+
plugs.push(plugins.allOf)
50+
}
51+
52+
// mapSpec is where the hard work happens, see https://github.com/swagger-api/specmap for more details
53+
return mapSpec({
54+
spec: _spec,
55+
context: {baseDoc},
56+
plugins: plugs,
57+
allowMetaPatches // allows adding .meta patches, which include adding `$$ref`s to the spec
58+
}).then(normalizeSwagger)
59+
}
5860
}

‎test/client.js‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ import fs from 'fs'
77
import Swagger from '../src/index'
88

99
describe('http', () => {
10-
var server
11-
before(function() {
10+
let server
11+
before(function () {
1212
server = http.createServer(function (req, res) {
13-
var accept = req.headers.accept
14-
var contentType
15-
var uri = url.parse(req.url).pathname;
16-
var filename = path.join('test', 'data', uri);
13+
const accept = req.headers.accept
14+
let contentType
15+
const uri = url.parse(req.url).pathname
16+
const filename = path.join('test', 'data', uri)
1717

18-
if(filename.indexOf('.yaml') > 0) {
18+
if (filename.indexOf('.yaml') > 0) {
1919
contentType = 'application/yaml'
2020
}
2121
else {
@@ -30,66 +30,66 @@ describe('http', () => {
3030
}
3131

3232
if (accept.indexOf('application/json') >= 0) {
33-
contentType = accept;
34-
res.setHeader('Content-Type', contentType);
33+
contentType = accept
34+
res.setHeader('Content-Type', contentType)
3535
}
3636
if (filename.indexOf('.yaml') > 0) {
37-
res.setHeader('Content-Type', 'application/yaml');
37+
res.setHeader('Content-Type', 'application/yaml')
3838
}
3939
}
4040

4141
fs.exists(filename, function (exists) {
42-
if(exists) {
43-
let fileStream = fs.createReadStream(filename);
42+
if (exists) {
43+
const fileStream = fs.createReadStream(filename)
4444
res.setHeader('Access-Control-Allow-Origin', '*')
4545
res.writeHead(200, contentType)
46-
fileStream.pipe(res);
46+
fileStream.pipe(res)
4747
}
4848
else {
49-
res.writeHead(404, {'Content-Type': 'text/plain'});
50-
res.write('404 Not Found\n');
51-
res.end();
49+
res.writeHead(404, {'Content-Type': 'text/plain'})
50+
res.write('404 Not Found\n')
51+
res.end()
5252
}
5353
})
5454
}).listen(8000)
5555
})
5656

57-
after(function() {
57+
after(function () {
5858
server.close()
5959
})
6060

6161
afterEach(function () {
6262
})
6363

64-
it.skip('should get the petstore api and build it', done => {
64+
it.skip('should get the petstore api and build it', (done) => {
6565
Swagger('http://localhost:8000/petstore.json')
66-
.then(client => {
66+
.then((client) => {
6767
expect(client).toExist()
6868

6969
// we have 3 tags
7070
expect(Object.keys(client.apis).length).toBe(3)
7171

7272
// the pet tag exists
73-
expect(client.apis['pet']).toExist()
73+
expect(client.apis.pet).toExist()
7474

7575
// the get pet operation
76-
expect(client.apis['pet'].getPetById).toExist()
76+
expect(client.apis.pet.getPetById).toExist()
7777

7878
done()
79-
})
79+
})
8080
})
8181

8282
/**
8383
* See https://github.com/swagger-api/swagger-js/issues/1005
8484
*/
85-
it.skip('should get a pet from the petstore', done => {
85+
it.skip('should get a pet from the petstore', (done) => {
8686
Swagger('http://localhost:8000/petstore.json')
87-
.then(client => {
88-
client.apis['pet'].getPetById({petId: -1})
89-
.then(data => {
87+
.then((client) => {
88+
client.apis.pet.getPetById({petId: -1})
89+
.then((data) => {
9090
done('shoulda thrown an error!')
9191
})
92-
.catch(err => {
92+
.catch((err) => {
9393
done()
9494
})
9595
})
@@ -98,24 +98,24 @@ describe('http', () => {
9898
/**
9999
* See https://github.com/swagger-api/swagger-js/issues/1002
100100
*/
101-
it.skip('should return an error when a spec doesnt exist', done => {
101+
it.skip('should return an error when a spec doesnt exist', (done) => {
102102
Swagger('http://localhost:8000/absent.yaml')
103-
.then(client => {
103+
.then((client) => {
104104
done('expected an error')
105105
})
106-
.catch(error => {
106+
.catch((error) => {
107107
done()
108108
})
109109
})
110110

111111
/**
112112
* See https://github.com/swagger-api/swagger-js/issues/1004
113113
*/
114-
it.skip('fail with invalid verbs', done => {
114+
it.skip('fail with invalid verbs', (done) => {
115115
Swagger('http://localhost:8000/invalid-operation.yaml')
116-
.then(client => {
117-
expect(client.apis['default']).toExist()
118-
expect(client.apis['default']['not-a-valid-verb']).toBeAn('undefined')
116+
.then((client) => {
117+
expect(client.apis.default).toExist()
118+
expect(client.apis.default['not-a-valid-verb']).toBeAn('undefined')
119119
done()
120120
})
121121
})
@@ -124,16 +124,16 @@ describe('http', () => {
124124
* Loads a spec where the `host` and `schema` are not defined
125125
* See https://github.com/swagger-api/swagger-js/issues/1000
126126
*/
127-
it('use the host from whence the spec was fetched', done => {
127+
it('use the host from whence the spec was fetched', (done) => {
128128
Swagger('http://localhost:8000/pathless.yaml')
129-
.then(client => {
130-
client.apis['default'].tryMe().then(data => {
131-
expect(data.status).toBe(404)
129+
.then((client) => {
130+
client.apis.default.tryMe().catch((err) => {
131+
expect(err.status).toBe(404)
132132
done()
133133
})
134134
})
135-
.catch(err => {
135+
.catch((err) => {
136136
done(err)
137137
})
138138
})
139-
})
139+
})

‎test/index.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('constructor', () => {
1313
})
1414

1515
it('should return an instance, without "new"', function (cb) {
16-
Swagger().then((instance) => {
16+
Swagger({spec: {}}).then((instance) => {
1717
expect(instance).toBeA(Swagger)
1818
cb()
1919
})
@@ -94,6 +94,21 @@ describe('constructor', () => {
9494
})
9595

9696
describe('#http', function () {
97+
it('should throw if fetch error', function (cb) {
98+
const xapp = xmock()
99+
xapp.get('http://petstore.swagger.io/404', (req, res) => {
100+
res.status(404)
101+
res.send('not found')
102+
})
103+
104+
new Swagger({url: 'http://petstore.swagger.io/404'})
105+
.catch((err) => {
106+
expect(err.status).toBe(404)
107+
expect(err.message).toBe('Not Found')
108+
cb()
109+
})
110+
})
111+
97112
it('should serialize the response', function () {
98113
// Given
99114
require('isomorphic-fetch') // To ensure global.Headers

0 commit comments

Comments
 (0)