From 1a6f29158422509bedc719aa6d6384551ddee51e Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Thu, 16 May 2019 18:58:48 +0530 Subject: [PATCH 1/6] Update stream-to-json-value.js Better error reporting in case of `json.parse` failures --- src/utils/stream-to-json-value.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/stream-to-json-value.js b/src/utils/stream-to-json-value.js index e42de2fc6..2ae83e50d 100644 --- a/src/utils/stream-to-json-value.js +++ b/src/utils/stream-to-json-value.js @@ -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) From 1d5e41e0775e93709098f45c5b991f8359a9d1be Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Thu, 16 May 2019 21:33:05 +0530 Subject: [PATCH 2/6] parseError taking non-JSON into account Making the `parseError` function compatible with non-JSON responses from server. - `isJson = true` parameter added that is backwards compatible with existing code Ref: https://github.com/ipfs/js-ipfs-http-client/issues/1000#issuecomment-493087784 --- src/utils/send-request.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/utils/send-request.js b/src/utils/send-request.js index 219327adf..82e7aee7c 100644 --- a/src/utils/send-request.js +++ b/src/utils/send-request.js @@ -13,9 +13,21 @@ const log = require('debug')('ipfs-http-client:request') // -- Internal -function parseError (res, cb) { +function parseError (res, cb, isJson = true) { const error = new Error(`Server responded with ${res.statusCode}`) + if (!isJson) { + return streamToValue(res, (err, data) => { + // 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) { + error.message = data.toString() + } + cb(error) + }) + } + streamToJsonValue(res, (err, payload) => { if (err) { return cb(err) @@ -44,7 +56,7 @@ function onRes (buffer, cb) { } if (res.statusCode >= 400 || !res.statusCode) { - return parseError(res, cb) + return parseError(res, cb, isJson) } // Return the response stream directly From d2d970d6e877c81204fe041f5586ab9765674b67 Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Fri, 17 May 2019 04:49:38 +0530 Subject: [PATCH 3/6] data should be non-empty (in addition to being non-null) `parseError` method should print default error in case data is null or empty ("") Co-Authored-By: Alan Shaw --- src/utils/send-request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/send-request.js b/src/utils/send-request.js index 82e7aee7c..8fe5a6f90 100644 --- a/src/utils/send-request.js +++ b/src/utils/send-request.js @@ -21,7 +21,7 @@ function parseError (res, cb, isJson = true) { // 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) { + if (data && data.length) { error.message = data.toString() } cb(error) From ff6d63bf7845b0b6cdb1f903a46cfd55f2e3cdd3 Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Fri, 17 May 2019 19:45:11 +0530 Subject: [PATCH 4/6] Moved isJson identification into a function This makes `parseError` more self-sufficient. --- src/utils/send-request.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/utils/send-request.js b/src/utils/send-request.js index 8fe5a6f90..a3a6afafc 100644 --- a/src/utils/send-request.js +++ b/src/utils/send-request.js @@ -13,10 +13,15 @@ const log = require('debug')('ipfs-http-client:request') // -- Internal -function parseError (res, cb, isJson = true) { +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}`) - if (!isJson) { + if (!hasJSONHeaders(res)) { return streamToValue(res, (err, data) => { // the `err` here refers to errors in stream processing, which // we ignore here, since we already have a valid `error` response @@ -46,8 +51,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) @@ -56,7 +60,7 @@ function onRes (buffer, cb) { } if (res.statusCode >= 400 || !res.statusCode) { - return parseError(res, cb, isJson) + return parseError(res, cb) } // Return the response stream directly From aaef3cb7a39334a23796541f5fcd71ff45d21507 Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Fri, 17 May 2019 19:55:34 +0530 Subject: [PATCH 5/6] removing trailing-space as per the lint 35:1 error Trailing spaces not allowed no-trailing-spaces --- src/utils/send-request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/send-request.js b/src/utils/send-request.js index a3a6afafc..20212f815 100644 --- a/src/utils/send-request.js +++ b/src/utils/send-request.js @@ -32,7 +32,7 @@ function parseError (res, cb) { cb(error) }) } - + streamToJsonValue(res, (err, payload) => { if (err) { return cb(err) From e79b7119d63336a3b9976d2e83dfdfa55a1cf133 Mon Sep 17 00:00:00 2001 From: Gopalakrishna Palem Date: Tue, 21 May 2019 16:47:29 +0530 Subject: [PATCH 6/6] adding status code to the error This is needed for the callers to know when the error needs a retry (such as 401, 403 etc. --- src/utils/send-request.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/send-request.js b/src/utils/send-request.js index 20212f815..b9f850561 100644 --- a/src/utils/send-request.js +++ b/src/utils/send-request.js @@ -29,6 +29,7 @@ function parseError (res, cb) { if (data && data.length) { error.message = data.toString() } + error.code = res.statusCode cb(error) }) }