tomas / needle

Nimble, streamable HTTP client for Node.js. With proxy, iconv, cookie, deflate & multipart support.
https://www.npmjs.com/package/needle
MIT License
1.63k stars 236 forks source link

HTTPS requests stuck on Node 10 & 11 #266

Open Borewit opened 6 years ago

Borewit commented 6 years ago

I have simple unit test fetching a file using HTTPS which runs fine on

but it fails on:

It is not specific to needle, I tried the following solutions and they all give the same result:

Reproduction of issue (written as a Mocha unit test) with Needle:

import { assert } from 'chai';
import * as needle from 'needle';
import * as Stream from 'stream';

import * as mm from '../lib';
import { IOptions } from '../src/type';

interface IHttpResponse {
  headers: { [id: string]: string; }
  stream: Stream.Readable;
}

interface IHttpClient {
  readonly name: string;
  get: (url: string) => Promise<IHttpResponse>;
}

class NeedleClient implements IHttpClient {

  public get(url: string): Promise<IHttpResponse> {
    return new Promise<IHttpResponse>((resolve, reject) => {
      const stream = needle.get(url);
      stream.on('headers', headers => {
        resolve({stream: stream as any, headers});
      });
      stream.on('err', err => {
        reject(err);
      });
    });
  }
}

const clients: IHttpClient[] = [

  {
    name: 'needle',
    client: new NeedleClient()
  }

];

describe('HTTP streaming', function() {

  // Increase time-out to 15 seconds because we retrieve files over HTTP(s)
  this.timeout(15 * 1000);
  this.retries(3); // Workaround for HTTP time-outs on Travis-CI

  describe('Stream HTTP using different clients', () => {

    clients.forEach(client => {

      describe(`HTTP client: ${client.name}`, () => {

        [true, false].forEach(hasContentLength => {

          it(`Should be able to parse M4A ${hasContentLength ? 'with' : 'without'} content-length specified`, () => {

            const url = 'https://tunalib.s3.eu-central-1.amazonaws.com/plan.m4a';

            return client.get(url).then(response => {

              const options: IOptions = {};
              if (hasContentLength) {
                options.fileSize = parseInt(response.headers['content-length'], 10); // Always pass this in production
              }

              return mm.parseStream(response.stream, response.headers['content-type'], options)
                .then(tags => {
                  if (response.stream.destroy) {
                    response.stream.destroy(); // Node >= v8 only
                  }
                  assert.strictEqual(tags.common.title, 'We Made a Plan');
                  assert.strictEqual(tags.common.artist, 'Adan Cruz');
                  assert.strictEqual(tags.common.album, 'Quiérelo');
                });
            });
          });

        });

      });
    });
  });

});

My current http test

Related issue on my end: https://github.com/Borewit/music-metadata/issues/160

Do you have any idea what the cause of the issue is?

tomas commented 6 years ago

Interesting. Have you tried with other servers?

Borewit commented 6 years ago

Good point, I did some more testing. Using the same sample, and installed Node v11 on my Windows machine (other results executed on Travis-CI):

Server Node v8.12.0 Node v11.0.0 Note
https://tunalib.s3.eu-central-1.amazonaws.com/plan.m4a ✔️ :x: Original location
https://dl.dropboxusercontent.com/... ✔️ :x: Public dropbox share
http://diskatation/... ✔️ :x: HTTP, intranet, Apache server

If I replace the sample with a 0 byte file on the local Apache it does no longer get stuck.

dzcpy commented 5 years ago

I have experienced some similar issues, although not with needle but x-ray, which uses superagent internally. Some connections got stuck for around 5 minutes, but I was using a socks5 proxy. I'm using Windows too, but i suspect that it might not be an issue on linux.