Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

fix: error reporting for non-JSON responses #1016

Merged
merged 8 commits into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/utils/send-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,26 @@ const log = require('debug')('ipfs-http-client:request')

// -- Internal

function hasJSONHeaders (res) {
return res.headers['content-type'] &&
res.headers['content-type'].indexOf('application/json') === 0
}

function parseError (res, cb) {
const error = new Error(`Server responded with ${res.statusCode}`)
error.statusCode = res.statusCode

if (!hasJSONHeaders(res)) {
return streamToValue(res, (err, data) => { // eslint-disable-line handle-callback-err
// the `err` here refers to errors in stream processing, which
// we ignore here, since we already have a valid `error` response
// from the server above that we have to report to the caller.
if (data && data.length) {
error.message = data.toString()
}
cb(error)
})
}

streamToJsonValue(res, (err, payload) => {
if (err) {
Expand All @@ -34,8 +52,7 @@ function onRes (buffer, cb) {
return (res) => {
const stream = Boolean(res.headers['x-stream-output'])
const chunkedObjects = Boolean(res.headers['x-chunked-output'])
const isJson = res.headers['content-type'] &&
res.headers['content-type'].indexOf('application/json') === 0
const isJson = hasJSONHeaders(res)

if (res.req) {
log(res.req.method, `${res.req.getHeaders().host}${res.req.path}`, res.statusCode, res.statusMessage)
Expand Down
2 changes: 1 addition & 1 deletion src/utils/stream-to-json-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function streamToJsonValue (res, cb) {
try {
res = JSON.parse(data)
} catch (err) {
return cb(err)
return cb(new Error(`Invalid JSON: ${data}`))
}

cb(null, res)
Expand Down
74 changes: 74 additions & 0 deletions test/request-api.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,77 @@ describe('trailer headers', () => {
})
})
})

describe('error handling', () => {
it('should handle plain text error response', function (done) {
if (!isNode) return this.skip()

const server = require('http').createServer((req, res) => {
// Consume the entire request, before responding.
req.on('data', () => {})
req.on('end', () => {
// Write a text/plain response with a 403 (forbidden) status
res.writeHead(403, { 'Content-Type': 'text/plain' })
res.write('ipfs method not allowed')
res.end()
})
})

server.listen(6001, () => {
ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => {
expect(err).to.exist()
expect(err.statusCode).to.equal(403)
expect(err.message).to.equal('ipfs method not allowed')
server.close(done)
})
})
})

it('should handle JSON error response', function (done) {
if (!isNode) return this.skip()

const server = require('http').createServer((req, res) => {
// Consume the entire request, before responding.
req.on('data', () => {})
req.on('end', () => {
// Write a application/json response with a 400 (bad request) header
res.writeHead(400, { 'Content-Type': 'application/json' })
res.write(JSON.stringify({ Message: 'client error', Code: 1 }))
res.end()
})
})

server.listen(6001, () => {
ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => {
expect(err).to.exist()
expect(err.statusCode).to.equal(400)
expect(err.message).to.equal('client error')
expect(err.code).to.equal(1)
server.close(done)
})
})
})

it('should handle JSON error response with invalid JSON', function (done) {
if (!isNode) return this.skip()

const server = require('http').createServer((req, res) => {
// Consume the entire request, before responding.
req.on('data', () => {})
req.on('end', () => {
// Write a application/json response with a 400 (bad request) header
res.writeHead(400, { 'Content-Type': 'application/json' })
res.write('{ Message: ')
res.end()
})
})

server.listen(6001, () => {
ipfsClient('/ip4/127.0.0.1/tcp/6001').config.replace('test/fixtures/r-config.json', (err) => {
expect(err).to.exist()
expect(err.message).to.include('Invalid JSON')
server.close(done)
})
})
})
})