Conversation
| const options = getNodeRequestOptions(request); | ||
|
|
||
| const send = (options.protocol === 'https:' ? https : http).request; | ||
| const send = (options.protocol === 'https:' ? http2 : http.request); |
There was a problem hiding this comment.
@devsnek Yes, I tried replacing both https & http modules with just http2-client, but some tests are failing then (mainly these related to error handling and form-data). I'm not sure about the form-data, but in terms of error handling - we would probably need to change the way we capture fetch errors in order to make it work:
207 passing (13s)
7 failing
1) node-fetch
should reject with error on network failure:
Uncaught Error: connect ECONNREFUSED 127.0.0.1:50000
at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)
2) node-fetch
system error is extracted from failed requests:
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/akepinski/dev/node-fetch/test/main.js)
at listOnTimeout (internal/timers.js:549:17)
at processTimers (internal/timers.js:492:7)
3) node-fetch
should handle DNS-error response:
Uncaught Error: getaddrinfo ENOTFOUND domain.invalid
at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:64:26)
4) node-fetch
should allow POST request with form-data as body:
FetchError: request to http://localhost:30001/multipart failed, reason: socket hang up
at ClientRequest.<anonymous> (src/index.js:107:11)
at Socket.socketOnEnd (_http_client.js:453:9)
at endReadableNT (_stream_readable.js:1187:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
5) node-fetch
should allow POST request with form-data using stream as body:
Uncaught Error: Message underflow.
at Parser._error (node_modules/parted/lib/multipart.js:317:7)
at Parser.end (node_modules/parted/lib/multipart.js:89:17)
at IncomingMessage.onend (_stream_readable.js:660:10)
at endReadableNT (_stream_readable.js:1187:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
6) node-fetch
should allow POST request with form-data using stream as body:
FetchError: request to http://localhost:30001/multipart failed, reason: socket hang up (and Mocha's done() called multiple times)
at ClientRequest.<anonymous> (src/index.js:107:11)
at Socket.socketOnEnd (_http_client.js:453:9)
at endReadableNT (_stream_readable.js:1187:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
7) node-fetch
should allow POST request with form-data as body and custom headers:
FetchError: request to http://localhost:30001/multipart failed, reason: socket hang up
at ClientRequest.<anonymous> (src/index.js:107:11)
at Socket.socketOnEnd (_http_client.js:453:9)
at endReadableNT (_stream_readable.js:1187:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
|
I'm strongly 👎 over this approach: not sure as for everybody else, but at least for me personally, |
|
@tinovyatkin I agree, I tried using |
Do not merge yet!
Fixes #342
I've decided to try and implement http2 support in node-fetch. I ended up using the
http2-clientmodule, which makes it easy to supporthttps/1.1andhttp/2.0protocols without doing any changes at all.The current implementation does not support unencrypted http2, due to test issues.
Now, I'm not sure whether this approach is correct. I will try to experiment more in the upcoming days.