Consider switching http libs

[turt2live]

something like https://github.com/visionmedia/superagent works nicely in both the browser and in node. request does work in both, but is painful and large for the browser (and causes weird problems sometimes).

[turt2live]

Related: #244

[jaller94]

Yes, ´superagent´ is a lot smaller than ´request´.

https://bundlephobia.com/result?p=request@2.88.0
https://bundlephobia.com/result?p=superagent@4.1.0

On GitHub ´superagent´ is currently labelled as passively maintained and looking for maintainers. However, this might not be a concern, as the library seems stable and well-developed at first glance.

As far as I can tell, a replacement of the dependency requires a mock of superagent. require is currently mocked by matrix-mock-request which you developed.

Do you have a list of steps which are required to give the experiment of replacing request a try?

[turt2live]

Replacing request would be a huge undertaking as the js-sdk and its consumers make a huge number of assumptions about the library being used. We'd almost certainly need to cut a major release, although that's not impossible.

Most of the HTTP code should be in one place, with the exception of all the places that rely on the response/error objects from request. Riot would also need to make use of the new library.

The shorter answer is that I unfortunately don't have a list, but I do know it is a long one.

[stalniy]

I would recommend to use axios. request is maintance mode

The most valuable thing request can do for the JavaScript ecosystem is to go into maintenance mode and stop considering new features or major releases

[jonassmedegaard]

Maybe this feature comparison is helpful in deciding which alternative to switch to: https://github.com/sindresorhus/got#comparison

[ghost]

I was having some issues login in with JWT token and request.

const user2 = await client.login("org.matrix.login.jwt", { token });

I ended up doing this for it to work:

const client = sdk.createClient({
      baseUrl: 'https://matrix-dev.mydomain.com',
      request: async function (opts, fn) {                                                                                                        
        opts.url = opts.uri;  
        opts.data = opts.body;  
        const res = await axios(opts);  
        fn(null, res, JSON.stringify(res.data));  
      },  
    });

Since I was not able to make this endpoint work with request in any way, I even tried isolated tests and I always received 'Error: Server returned 403 error' until I switched to axios, this endpoint is working with browser-request.

Since it seems like a very simple change in interface from request to axios, is there any effort already going on switching into another library?

I would love to contribute to this if you think this is the right approach.

[t3chguy]

There is a WIP PR to move over to fetch, which is a more likely candidate as it is built-in in the web

[ghost]

Great! Thanks.

[bradjones1]

This is the aforementioned PR, though it is 4 years old now: #797

[bradjones1]

Not sure if this helps in evaluation, but I had a use case to use Fetch with the current SDK; here's a gist of my wrapped implementation that supports aborting. Seems to work... so far?

https://gist.github.com/bradjones1/dae3207aa7dce6eb508cfec56a76ae99

[MadLittleMods]

Closing in favor of element-hq/element-web#22265 which has had recent push to be part of the 2022 tech strategy.

[t3chguy]

@MadLittleMods given that element-web is not the only consumer of the js-sdk, this is a far better place to collect opinions from other consumers

[t3chguy]

Keeping Enhancement due to it adding new features

• We won't be depending on an unmaintained library
• We think this will allow us to upload files without loading them into memory, as File objects can be passed as a Blob in the body option of a Request rather than having to load blob data into a buffer as is currently done
• We could use the priority option to make some requests more important than others

[t3chguy]

Current thinking is to switch to fetch, for js-sdk it'll still need a polyfill given the Node version where fetch support was added is more modern than our minimum supported version

[3nprob]

Not sure if OT for this topic or not but it'd be great if a replacing lib either has or is easy to extend with reasonable (exponential backoffs) retry behavior. I think this has the potential to improve UX greatly in scenarios with unreliable or intermittent HS connectivity.

The fetch API has the obvious benefit of being native and could be wrapped with something like @adobe/node-fetch-retry. Note that as of v18, nodeJS native fetch functionality is provided by undici, which has some subtle differences compared with node-fetch so using that instead of node-fetch will minimize surprising behavioral differences between node versions while leveraging the native API when possible.

For a nodejs-only app, got has IME been the unchallenged champion for some time now. So far I still haven't had the need to debug it even once, which says something. Not really relevant here, though, unless implementing a new wrapper is an option, as it's not API-compatible with fetch.

Since axios was mentioned: Even if axios is popular and looks great at first glance we had so many tricky-to-debug issues and edge-cases with it over the years, some are still open. I really would not recommend a project migrating to it today.

[t3chguy]

easy to extend with reasonable (exponential backoffs) retry behavior.

Retry behaviour is rarely easy, given that even if the response failed to make it to the requester, the effect may have taken place, so really you can only retry GET and PUTs which are idempotent via txnId

[3nprob]

easy to extend with reasonable (exponential backoffs) retry behavior.

Retry behaviour is rarely easy, given that even if the response failed to make it to the requester, the effect may have taken place, so really you can only retry GET and PUTs which are idempotent via txnId

Depends. Common practice is to differentiate between error codes, where e.g. 408, 502, 503, 504, and potentially 429 are almost always safe to retry.

[t3chguy]

408 Request Timeout doesn't feel safe to retry, the server may have received the request successfully here and acted upon it.

---

502 Bad Gateway doesn't feel safe to retry, given it is specified as

the server, while acting as a gateway or proxy, received an invalid response from the upstream server.

Not that the upstream server wasn't given the request

---

504 Gateway Timeout is certainly not safe to retry, in this case the upstream server more than likely got the request, but the response will never make it back to the caller. This is the error you'll see on matrix.org when joining a room takes too long for example.

[3nprob]

Honestly even limiting retries to just idempotent GETs would still cover the instances I can recall where this forced me to restart element-desktop or rendered the whole app unoperable, so I still think that's a win.