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

refactor: replace wreck with raw request #414

Merged
merged 3 commits into from
Nov 8, 2016
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
14 changes: 11 additions & 3 deletions examples/browser-add/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,23 @@
<meta charset="UTF-8"/>
<title>JS IPFS API - Example - Browser - Add</title>
<script src="bundle.js"></script>
<style>
.content {
border: 1px solid black;
padding: 10px;
margin: 5px 0;
}
</style>
</head>
<body>
<h1>JS IPFS API - Add file from the browser</h1>
<textarea id="source">
</textarea>
<button id="store">create in ipfs</button>
<div><div>found in ipfs:</div>
<div id="hash">[ipfs hash]</div>
<div id="content">[ipfs content]</div>
<div>
<div>found in ipfs:</div>
<div class="content" id="hash">[ipfs hash]</div>
<div class="content" id="content">[ipfs content]</div>
</div>
</body>
</html>
24 changes: 13 additions & 11 deletions examples/browser-add/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict'

var IPFS = require('ipfs-api')
var IPFS = require('../../src')
var ipfs = IPFS()

function store () {
Expand All @@ -11,24 +11,26 @@ function store () {
}

res.forEach(function (file) {
console.log('successfully stored', file.Hash)
display(file.Hash)
if (file && file.hash) {
console.log('successfully stored', file.hash)
display(file.hash)
}
})
})
}

function display (hash) {
ipfs.cat(hash, function (err, res) {
// buffer: true results in the returned result being a buffer rather than a stream
ipfs.cat(hash, {buffer: true}, function (err, res) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be buffered?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this buffer: true is one of the most misleading features. Can you add a comment that says:
"Buffer the whole response instead of streaming it"

if (err || !res) {
return console.error('ipfs cat error', err, res)
}
if (res.readable) {
console.error('unhandled: cat result is a pipe', res)
} else {
document.getElementById('hash').innerText = hash
document.getElementById('content').innerText = res
}

document.getElementById('hash').innerText = hash
document.getElementById('content').innerText = res.toString()
})
}

document.getElementById('store').onclick = store
document.addEventListener('DOMContentLoaded', function () {
document.getElementById('store').onclick = store
})
6 changes: 2 additions & 4 deletions examples/browser-add/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
"description": "",
"main": "index.js",
"scripts": {
"start": "browserify -t brfs index.js > bundle.js && http-server -a 127.0.0.1 -p 8888"
"start": "browserify index.js > bundle.js && http-server -a 127.0.0.1 -p 8888"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

},
"keywords": [],
"author": "Friedel Ziegelmayer",
"license": "MIT",
"devDependencies": {
"brfs": "^1.4.3",
"browserify": "^13.0.1",
"http-server": "^0.9.0",
"ipfs-api": "^6.0.3"
"http-server": "^0.9.0"
}
}
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"browser": {
"glob": false,
"fs": false,
"stream": "readable-stream"
"stream": "readable-stream",
"http": "stream-http"
},
"scripts": {
"test": "gulp test",
Expand All @@ -24,6 +25,7 @@
"async": "^2.1.2",
"bl": "^1.1.2",
"bs58": "^3.0.0",
"concat-stream": "^1.5.2",
"detect-node": "^2.0.3",
"flatmap": "0.0.3",
"glob": "^7.1.1",
Expand All @@ -40,9 +42,9 @@
"promisify-es6": "^1.0.2",
"qs": "^6.3.0",
"readable-stream": "^1.1.14",
"stream-http": "^2.5.0",
"streamifier": "^0.1.1",
"tar-stream": "^1.5.2",
"wreck": "^10.0.0"
"tar-stream": "^1.5.2"
},
"engines": {
"node": ">=4.0.0"
Expand Down
10 changes: 8 additions & 2 deletions src/api/cat.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ const promisify = require('promisify-es6')
const cleanMultihash = require('../clean-multihash')

module.exports = (send) => {
return promisify((hash, callback) => {
return promisify((hash, opts, callback) => {
if (typeof opts === 'function') {
callback = opts
opts = {}
}

try {
hash = cleanMultihash(hash)
} catch (err) {
Expand All @@ -13,7 +18,8 @@ module.exports = (send) => {

send({
path: 'cat',
args: hash
args: hash,
buffer: opts.buffer
}, callback)
})
}
18 changes: 11 additions & 7 deletions src/api/util/url-add.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
'use strict'

const Wreck = require('wreck')
const addToDagNodesTransform = require('./../../add-to-dagnode-transform')

const promisify = require('promisify-es6')
const once = require('once')
const parseUrl = require('url').parse

const request = require('../../request')
const addToDagNodesTransform = require('./../../add-to-dagnode-transform')

module.exports = (send) => {
return promisify((url, opts, callback) => {
Expand All @@ -27,17 +29,19 @@ module.exports = (send) => {
}

const sendWithTransform = send.withTransform(addToDagNodesTransform)
callback = once(callback)

Wreck.request('GET', url, null, (err, res) => {
if (err) {
return callback(err)
request(parseUrl(url).protocol)(url, (res) => {
res.once('error', callback)
if (res.statusCode >= 400) {
return callback(new Error(`Failed to download with ${res.statusCode}`))
}

sendWithTransform({
path: 'add',
qs: opts,
files: res
}, callback)
})
}).end()
})
}
98 changes: 65 additions & 33 deletions src/request-api.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,54 @@
'use strict'

const Wreck = require('wreck')
const Qs = require('qs')
const ndjson = require('ndjson')
const getFilesStream = require('./get-files-stream')

const isNode = require('detect-node')
const once = require('once')
const concat = require('concat-stream')

const getFilesStream = require('./get-files-stream')
const request = require('./request')

// -- Internal

function parseChunkedJson (res, cb) {
const parsed = []
res
.pipe(ndjson.parse())
.on('data', (obj) => {
parsed.push(obj)
})
.on('end', () => {
cb(null, parsed)
})
.once('error', cb)
.pipe(concat((data) => cb(null, data)))
}

function onRes (buffer, cb, uri) {
return (err, res) => {
if (err) {
return cb(err)
}
function parseRaw (res, cb) {
res
.once('error', cb)
.pipe(concat((data) => cb(null, data)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use bl?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because concat does the right thing for streams, arrays and buffers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you :) Will remind me of that the next time I need to do this

}

function parseJson (res, cb) {
res
.once('error', cb)
.pipe(concat((data) => {
if (!data || data.length === 0) {
return cb()
}

if (Buffer.isBuffer(data)) {
data = data.toString()
}

let res
try {
res = JSON.parse(data)
} catch (err) {
return cb(err)
}

cb(null, res)
}))
}

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'] &&
Expand All @@ -35,7 +57,7 @@ function onRes (buffer, cb, uri) {
if (res.statusCode >= 400 || !res.statusCode) {
const error = new Error(`Server responded with ${res.statusCode}`)

return Wreck.read(res, {json: true}, (err, payload) => {
parseJson(res, (err, payload) => {
if (err) {
return cb(err)
}
Expand All @@ -51,20 +73,21 @@ function onRes (buffer, cb, uri) {
return cb(null, res)
}

if (chunkedObjects) {
if (isJson) {
return parseChunkedJson(res, cb)
}
if (chunkedObjects && isJson) {
return parseChunkedJson(res, cb)
}

return Wreck.read(res, null, cb)
if (isJson) {
return parseJson(res, cb)
}

Wreck.read(res, {json: isJson}, cb)
parseRaw(res, cb)
}
}

function requestAPI (config, options, callback) {
options.qs = options.qs || {}
callback = once(callback)

if (Array.isArray(options.files)) {
options.qs.recursive = true
Expand Down Expand Up @@ -99,29 +122,38 @@ function requestAPI (config, options, callback) {
// this option is only used internally, not passed to daemon
delete options.qs.followSymlinks

const port = config.port ? `:${config.port}` : ''

const opts = {
method: 'POST',
uri: `${config.protocol}://${config.host}${port}${config['api-path']}${options.path}?${Qs.stringify(options.qs, {arrayFormat: 'repeat'})}`,
headers: {}
}
const method = 'POST'
const headers = {}

if (isNode) {
// Browsers do not allow you to modify the user agent
opts.headers['User-Agent'] = config['user-agent']
headers['User-Agent'] = config['user-agent']
}

if (options.files) {
if (!stream.boundary) {
return callback(new Error('No boundary in multipart stream'))
}

opts.headers['Content-Type'] = `multipart/form-data; boundary=${stream.boundary}`
opts.payload = stream
headers['Content-Type'] = `multipart/form-data; boundary=${stream.boundary}`
}

const qs = Qs.stringify(options.qs, {arrayFormat: 'repeat'})
const req = request(config.protocol)({
hostname: config.host,
path: `${config['api-path']}${options.path}?${qs}`,
port: config.port,
method: method,
headers: headers
}, onRes(options.buffer, callback))

if (options.files) {
stream.pipe(req)
} else {
req.end()
}

return Wreck.request(opts.method, opts.uri, opts, onRes(options.buffer, callback, opts.uri))
return req
}

//
Expand Down
12 changes: 12 additions & 0 deletions src/request.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict'

const httpRequest = require('http').request
const httpsRequest = require('https').request

module.exports = (protocol) => {
if (protocol.indexOf('https') === 0) {
return httpsRequest
}

return httpRequest
}
Loading