You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently timeout is done with a Promise.race, where we have two promises, one for the fetch and one for a setTimeout.
Using this solution will leave the request hanging and still consume bandwidth in the background and lower the maximum allowed concurrent requests being made while it's still in process. (https://stackoverflow.com/a/50101022)
// TimeoutPromise will not run if undefined or zero
lettimeoutId: ReturnType<typeofsetTimeout>
if(timeout){
consttimeoutPromise=newPromise((_,reject)=>{
timeoutId=setTimeout(()=>{
reject(newError('Error: Request Timed Out'))
},timeout)
})
promises.push(timeoutPromise)
}
Promise.race(promises)
.then(resolve)
.catch(reject)
.finally(()=>{
clearTimeout(timeoutId)
})
})
}
Instead this timeout option should add an AbortSignal.timeout to the Request option's signal, if there isn't already one supplied. We should warn people, that if they're supplying this signal option themselves, their timeout will not work.
We could use AbortSignal.any to keep the same functionality as before, but it is not available in Node.js 18, and there's no polyfill for it either.
EDIT: Looks like AbortSignal.timeout has no polyfill either. In fact all of AbortController and AbortSignal isn't very well polyfilled, if at all. This would definitely have a big impact on compatibility, at least if one wishes to set a global timeout :\ . I really hate how the web is held back like this.
The text was updated successfully, but these errors were encountered:
Currently timeout is done with a
Promise.race
, where we have two promises, one for thefetch
and one for asetTimeout
.Using this solution will leave the request hanging and still consume bandwidth in the background and lower the maximum allowed concurrent requests being made while it's still in process. (https://stackoverflow.com/a/50101022)
meilisearch-js/src/http-requests.ts
Lines 177 to 208 in cd61a8c
Instead this
timeout
option should add anAbortSignal.timeout
to theRequest
option'ssignal
, if there isn't already one supplied. We should warn people, that if they're supplying thissignal
option themselves, theirtimeout
will not work.We could use
AbortSignal.any
to keep the same functionality as before, but it is not available in Node.js 18, and there's no polyfill for it either.EDIT: Looks like
AbortSignal.timeout
has no polyfill either. In fact all ofAbortController
andAbortSignal
isn't very well polyfilled, if at all. This would definitely have a big impact on compatibility, at least if one wishes to set a globaltimeout
:\ . I really hate how the web is held back like this.The text was updated successfully, but these errors were encountered: