Replaced request with node-fetch

[dandv]

node-tech for use with isomorphic-fetch in browsers, instead of got.

• Fix the node-fetch issue with 204 no-content
• Fix last test - All tests pass, except the last hacky one. What exactly does the second "this" refer to in the comment?

[connor4312]

I believe this refers to the maxRetries option in InfluxRequest

[connor4312]

Perhaps checking the statusCode for 204s would be cleaner here? I believe the issue on node-fetch would break away from the whatwg specification, which simply states that response.json() should

[r]eturn the result of invoking the initial value of the parse property of the JSON object with the result of running utf-8 decode on bytes as argument. Rethrow any exceptions.

[dandv]

That would be less resilient to imperfect implementations that return, say, 200 and an empty response, but it would help catch that error.

I've amended my PR to node-fetch to check for a status code of 204 instead.

[connor4312]

I agree it's less resilient, but as far as adding it in node-fetch:

• it is a breaking API change
• empty objects are not necessarily equivalent to empty responses
• it breaks away from the specification which node-fetch is trying to match (!)
• it also will not be a feature in browsers; even if github/fetch also accepts such a change, it won't be present in browsers on which the polyfill is not applied (modern Chrome/FireFox)

[dandv]

Shouldn't GitHub be hiding this discussion now that it refers to an outdated diff?

[connor4312]

After some review for integration with 5.x, I'm not totally sure that using fetch, particularly moving toward isomorphic fetch, is the right way to go yet. Two reasons:

• the de-facto standard for fetch in the browser is a global polyfill; generally I do not think that it is the place of dependencies to provide such polyfills
• node-fetch implements some useful features like timeouts, but these are not present in the browser version (or official spec) and they would have to be special-cased; personally I'd rather drop a dependency in favor of good ol' fashioned XHR if we would have to manually do detection regardless

It seems like using the http module directly is a reasonable way to go. Its API is decently friendly and there are quality compatible API implementations for browsers which will work out of the box in most loaders. (Note that Webpack currently uses an unmaintained compatibility layer which does not support timeouts, but Browserify has moved over and it looks like Webpack will soon)

[dandv]

Influent works in the browser by using a small HTTP module, hurl, which uses XHR.

[dandv]

FWIW, this branch works without problems in the browser, without isomorphic-fetch. I've tested with jspm 0.17, connecting over HTTPS to the InfluxDB server.

[connor4312]

Can you rebase on or back-merge in master? Happy to add this in now, though I still want to stick with with native http module, ponyfilled with stream-promise in Webpack/Browserify, in 5.0.0 :)

[dandv]

@connor4312: closing in favor of the #202 rebase.