nodeca / probe-image-size

Get image size without full download. Supported image types: JPG, GIF, PNG, WebP, BMP, TIFF, SVG, PSD, ICO.
MIT License
978 stars 77 forks source link

Fix bug when set agent for needle & add test cases #54

Closed taina0407 closed 3 years ago

taina0407 commented 3 years ago

Using deepmerge make agent object invalid with node http, using lodash will avoid this issue. Also added test case for using agent

puzrin commented 3 years ago

I would not like switch to lodash.merge. Could you explain details about problem (and minimal example how to replroduce)?

taina0407 commented 3 years ago

I would not like switch to lodash.merge. Could you explain details about problem (and minimal example how to replroduce)?

As I comment above, because the "agent" configuration instance will be convert to pure object when using deepmerge library, that will cause the needle plugin to fail when using agent (for example to keep alive TCP connection)

Here is the simple code for procedure this error:

const http = require("http");
const https = require("https");
const Probe = require("probe-image-size");

(async () => {
  try {
    await Probe("http://via.placeholder.com/350x150", {
      agent: new http.Agent({
        keepAlive: true,
      }),
    });
  } catch (error) {
    console.error(error);
  }

  try {
    await Probe("https://via.placeholder.com/350x150", {
      agent: new https.Agent({
        keepAlive: true,
      }),
    });
  } catch (error) {
    console.error(error);
  }
})();

These will throw exception (Tested with Node 12 and 14)

TypeError [ERR_INVALID_ARG_TYPE]: The "options.agent" property must be one of Agent-like Object, undefined, or false. Received an instance of Object
    at new ClientRequest (_http_client.js:133:11)
    at Object.request (http.js:46:10)
    at Needle.send_request (/FAKE_PATH/server/node_modules/needle/lib/needle.js:504:26)
    at next (/FAKE_PATH/server/node_modules/needle/lib/needle.js:391:10)
    at Needle.start (/FAKE_PATH/server/node_modules/needle/lib/needle.js:394:17)
    at Function.module.exports.<computed> [as get] (/FAKE_PATH/server/node_modules/needle/lib/needle.js:817:61)
    at /FAKE_PATH/server/node_modules/probe-image-size/http.js:31:23
    at new Promise (<anonymous>)
    at probeHttp (/FAKE_PATH/server/node_modules/probe-image-size/http.js:27:10)
    at get_image_size (/FAKE_PATH/server/node_modules/probe-image-size/index.js:17:10) {
  code: 'ERR_INVALID_ARG_TYPE'
}
puzrin commented 3 years ago

Got it. Seems deepmerge has desired changes in upcoming 5.x, but development is not active and we can't wait without deadlines.

Let's accept your proposal, and [may be] roll back later. Thanks for your efforts!

puzrin commented 3 years ago

Published 7.1.0