trezor / trezor-suite

Trezor Suite Monorepo
https://trezor.io/trezor-suite
Other
720 stars 251 forks source link

Remove cross-fetch lib as we use Node18 everywhere #10146

Open tomasklim opened 11 months ago

tomasklim commented 11 months ago
komret commented 8 months ago

Removed from several packages in https://github.com/trezor/trezor-suite/pull/11223.

Removing in from @trezor/coinjoin would break coordinatorRequest test because request fails - unlike if cross-fetch is used.

Removing it from @trezor/connect requires reworking the mocks.

peter-sanderson commented 6 months ago

Lets not do this before Node19. It has some differeneces and causing troubles. See: https://github.com/trezor/trezor-suite/pull/11820

(Details: https://satoshilabs.slack.com/archives/C02V2PSDNA2/p1711539811017599)

peter-sanderson commented 6 months ago

Here is also relevant Github Issue: https://github.com/nodejs/node/issues/46375

peter-sanderson commented 6 months ago

Also good read: https://betterstack.com/community/guides/scaling-nodejs/nodejs-timeouts/#timing-out-a-fetch-api-request

It says that in node fetch has no timeout, but it seems that its not true

In browsers, fetch() usually times out after a set period of time which varies amongst browsers. For example, in Firefox this timeout is set to 90 seconds by default, but in Chromium, it is 300 seconds. In Node.js, no default timeout is set for fetch() requests, but the newly added AbortSignal.timeout() API provides an easy way to cancel a fetch() request when a timeout is reached.

peter-sanderson commented 6 months ago

Finally another issue: https://github.com/nodejs/undici/issues/1373

TBH I dont see a good solution to this right now :(

peter-sanderson commented 6 months ago

Maybe a solution is to use undici. There is a way how to define timeouts globally.

const { Agent } = require('undici');

dispatcher = new Agent({
    connectTimeout: 10 * 60e3, // 10 minutes
    bodyTimeout: 10 * 60e3, // 10 minutes
    headersTimeout: 10 * 60e3, // 10 minutes
})

globalThis[Symbol.for('undici.globalDispatcher.1')] = dispatcher

// ----------------------------------------------------------------------------

const t = Date.now()

const run = async () => {
    try {
        await fetch("https://192.168.0.1");
    } finally {
        console.log((Date.now() - t) / 1000);
    }
}

run();

Source of the idea: https://github.com/nodejs/undici/issues/1373#issuecomment-1426025119

komret commented 6 months ago

It's not pressing, let's update node first and see if it works.