Replace "request" with something else

[bajtos]

request module is moving to maintenance mode, we should eventually find a new HTTP client.

• The announcement: Request’s Past, Present and Future request/request#3142
• List of alternatives: Alternative libraries to request request/request#3143
• Comparison: https://www.npmjs.com/package/got#comparison

I tried to upgrade to got. got works well for simple use cases, but stops working for non-trivial requests (e.g. when a proxy is involved). What's worse, it replaces errors reported by Node.js core with its own error classes, which makes troubleshooting extremely difficult 😞 It seems that other popular clients like node-fetch and axios are doing the same mistake 😞

(UPDATED on Jun 12, 2019)

Acceptance Criteria

• Review request alternatives (e.g. axios and node-fetch), find one that meets the following requirements:
  • It should preserve low-level stack traces reported by Node.js instead of thrown fresh error instances
  • It need to support proxies, ideally @loopback/http-caching-proxy too.
  • If it's browser-friendly, then we should choose something that's more compatible with node-fetch API.

It can be a reusable component/module where we can customize the behavior.

[seiya-git]

for proxy u can use https://www.npmjs.com/package/proxy-agent with got

[hacksparrow]

What about https://www.npmjs.com/package/axios?

Oh, you already mentioned axios.

[bajtos]

for proxy u can use https://www.npmjs.com/package/proxy-agent with got

Good to know! I was trying https://github.com/koichik/node-tunnel and that did not work.

In general, I am very reluctant to use a library that's discarding low-level stack traces reported by Node.js and replacing them with a new stack pointing only to the client library. Unfortunately, that rules out both got and axios.

[dhmlau]

@bajtos @hacksparrow, do you think we should turn this into a spike to find out what's a good substitution?

I'm thinking of the following acceptance criteria. Please review:

Acceptance Criteria

• Review the list of alternatives listed in Alternative libraries to request request/request#3143, and determine which one is the best
  • Question: what are the criteria to determine the most suited one?

[raymondfeng]

I don't have strong opinions here but axios and node-fetch seem to be two popular candidates.

[dhmlau]

Thanks @raymondfeng. How do we know which one is better? :) Any criteria to determine that?

[bajtos]

As of today, request has been officially deprecated at npm level.

[bajtos]

AFAICT, https://www.npmjs.com/package/needle should be preserving error details reported by Node.js core and supports Promise-style too. It may be a good alternative to consider.

[raymondfeng]

First step to replace request in loopback-next - #4628.

The usage of request in http-caching-proxy is more involved.

[bajtos]

First step to replace request in loopback-next - #4628.

@raymondfeng Can you please confirm that axios is preserving low-level error details? (See the discussion above.)

Ideally, I'd like us to use the same library across the entire loopback-next codebase, including connectors like REST and SOAP.

The usage of request in http-caching-proxy is more involved.

IMO, we should start with looking for a library that works well for http-caching-proxy and then use that one everywhere else.

[bajtos]

FYI, here is a chart comparing download numbers for different request alternatives: https://npmcharts.com/compare/axios,needle,node-fetch,got?interval=30

[bajtos]

I did a quick experiment with needle. I like the fact that it always preserves low-level Node.js error details and allows string-based proxy configuration (no need to parse the proxy URL). However, it has different implementation of timeout handling, which makes it difficult to pass the timeout-checking test in http-caching-proxy.

I quickly scanned the source code of node-fetch and got, and unfortunately it looks like they don't provide options for preserving error details from Node.js core :(

The good thing about Axios is that they should be preserving original Node.js errors, at least as far as I can tell - see https://github.com/axios/axios/blob/03e6f4bf4c1eced613cf60d59ef50b0e18b31907/lib/adapters/http.js#L224-L245

[dhmlau]

There are 2 references of the request module with https://www.npmjs.com/package/request in docs. We may want to update that as well:

1. https://loopback.io/doc/en/lb4/Calling-other-APIs-and-web-services.html#datasource-for-rest-service
2. https://github.com/strongloop/loopback-next/tree/master/packages/http-caching-proxy#basic-use

[raymondfeng]

There are 2 references of the request module with https://www.npmjs.com/package/request in docs. We may want to update that as well:
https://loopback.io/doc/en/lb4/Calling-other-APIs-and-web-services.html#datasource-for-rest-service

This comes from loopback-connector-rest. I'm not sure if we want to migrate to axios at this moment.

https://github.com/strongloop/loopback-next/tree/master/packages/http-caching-proxy#basic-use

Fixed.

[bajtos]

This comes from loopback-connector-rest. I'm not sure if we want to migrate to axios at this moment.

IMO, we need to migrate all actively-developer connectors to use Axios instead of request. Here is why:

$ npm i loopback-connector-rest
npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142

I am proposing to release that change in a new major version, to allow people to stick with request if they prefer. (Plus to avoid breaking behavior of existing application in edge cases where Axios and Request behave differently.)

[raymondfeng]

To be fixed:

• request module is deprecated strong-soap#262