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

Test failures on macOS w/ node v17 #393

Closed joeyparrish closed 2 years ago

joeyparrish commented 2 years ago

I get the following test failures on macOS.

These two because of a bad URL, and should fail on all platforms. They rely on http://www.nina.jp/server/slackware/webapp/tomcat_charset.html , but this no longer exists. I believe this should be replaced by a local server.

  1) character encoding
       Given content-type: "text/html; charset=EUC-JP"
         with decode = false
           does not decode:
     Uncaught TypeError: Cannot read properties of undefined (reading 'body')
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/decoder_spec.js:24:16
      at done (lib/needle.js:501:14)
      at ClientRequest.had_error (lib/needle.js:516:5)
      at ClientRequest.emit (node:events:526:28)
      at Socket.socketErrorListener (node:_http_client:442:9)
      at Socket.emit (node:events:526:28)
      at emitErrorNT (node:internal/streams/destroy:164:8)
      at emitErrorCloseNT (node:internal/streams/destroy:129:3)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)

  2) character encoding
       Given content-type: "text/html; charset=EUC-JP"
         with decode = true
           decodes:
     Uncaught TypeError: Cannot read properties of undefined (reading 'body')
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/decoder_spec.js:39:16
      at done (lib/needle.js:501:14)
      at ClientRequest.had_error (lib/needle.js:516:5)
      at ClientRequest.emit (node:events:526:28)
      at Socket.socketErrorListener (node:_http_client:442:9)
      at Socket.emit (node:events:526:28)
      at emitErrorNT (node:internal/streams/destroy:164:8)
      at emitErrorCloseNT (node:internal/streams/destroy:129:3)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)

These fail if run on macOS because they only exclude Windows, and /proc/self/fd isn't available on Mac, either. I believe that these should only run on Linux.

  3) with output option
       and a 200 response
         for an empty response
           closes the file descriptor:
     Error: ENOENT: no such file or directory, scandir '/proc/self/fd'
      at Object.readdirSync (node:fs:1392:3)
      at get_open_file_descriptors (test/output_spec.js:27:19)
      at Context.<anonymous> (test/output_spec.js:114:34)
      at processImmediate (node:internal/timers:466:21)

  4) with output option
       and a 200 response
         for a JSON response
           closes the file descriptor:
     Error: ENOENT: no such file or directory, scandir '/proc/self/fd'
      at Object.readdirSync (node:fs:1392:3)
      at get_open_file_descriptors (test/output_spec.js:27:19)
      at Context.<anonymous> (test/output_spec.js:171:32)
      at processImmediate (node:internal/timers:466:21)

  5) with output option
       and a 200 response
         for a binary file
           closes the file descriptor:
     Error: ENOENT: no such file or directory, scandir '/proc/self/fd'
      at Object.readdirSync (node:fs:1392:3)
      at get_open_file_descriptors (test/output_spec.js:27:19)
      at Context.<anonymous> (test/output_spec.js:241:34)
      at processImmediate (node:internal/timers:466:21)

These tests seem to be testing some behavior of node's HTTP server, not the client. The client doesn't control if the server returns 400 (expected on node >= 10), or ECONNRESET (expected on other versions), and therefore fails if the server behaves differently than expected (returns 200 for me). Since the only expectations are set on a component outside this project, I believe this is just a bad test that should be deleted.

  6) request stream length
       no stream_length set
         returns 400 if Transfer-Encoding is set to a blank string:

      Uncaught AssertionError: expected 200 to equal 400
      + expected - actual

      -200
      +400

      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/request_stream_spec.js:66:34
      at done (lib/needle.js:501:14)
      at PassThrough.<anonymous> (lib/needle.js:764:9)
      at PassThrough.emit (node:events:538:35)
      at endReadableNT (node:internal/streams/readable:1342:12)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)

  7) request stream length
       stream_length set to invalid value
         returns 400 if Transfer-Encoding is set to a blank string:

      Uncaught AssertionError: expected 200 to equal 400
      + expected - actual

      -200
      +400

      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/request_stream_spec.js:96:34
      at done (lib/needle.js:501:14)
      at PassThrough.<anonymous> (lib/needle.js:764:9)
      at PassThrough.emit (node:events:538:35)
      at endReadableNT (node:internal/streams/readable:1342:12)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)

  8) request stream length
       stream_length set to invalid value
         returns 400 if Transfer-Encoding is not set:

      Uncaught AssertionError: expected 200 to equal 400
      + expected - actual

      -200
      +400

      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/request_stream_spec.js:104:34
      at done (lib/needle.js:501:14)
      at PassThrough.<anonymous> (lib/needle.js:764:9)
      at PassThrough.emit (node:events:538:35)
      at endReadableNT (node:internal/streams/readable:1342:12)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)

  9) request stream length
       stream_length set to 0
         stream without path
           returns 400 if Transfer-Encoding is set to a blank string:

      Uncaught AssertionError: expected 200 to equal 400
      + expected - actual

      -200
      +400

      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at /Users/alcatrazqa/joey/shaka-player/node_modules/needle/test/request_stream_spec.js:213:36
      at done (lib/needle.js:501:14)
      at PassThrough.<anonymous> (lib/needle.js:764:9)
      at PassThrough.emit (node:events:538:35)
      at endReadableNT (node:internal/streams/readable:1342:12)
      at processTicksAndRejections (node:internal/process/task_queues:83:21)
joeyparrish commented 2 years ago

I should have mentioned, these failures were all on node v17.