Fetch with a timeout everywhere #10101
Conversation
Builds ready [35f589d]
Page Load Metrics (534 ± 43 ms)
|
| @@ -227,7 +225,7 @@ export default class IncomingTransactionsController { | |||
| if (fromBlock) { | |||
| url += `&startBlock=${parseInt(fromBlock, 10)}` | |||
| } | |||
| const response = await fetch(url) | |||
| const response = await fetchWithTimeout(url) | |||
There was a problem hiding this comment.
Do we want to add a try/catch here?
There was a problem hiding this comment.
What would we do in the catch block in these cases?
There was a problem hiding this comment.
Log an error to Sentry so we can see if we're having transaction loading errors? Or would that not be helpful?
There was a problem hiding this comment.
These already get logged to Sentry, by virtue of being uncaught. Uncaught errors are all logged to Sentry.
Edit: Oh, actually, no these are caught. They're caught and logged to the console a few levels up in _getDataForUpdate.
Though even still, I don't think this is necessarily something we want to log to Sentry anyway, because failures here aren't actionable for us (e.g. it's not a bug if someone disconnects from the internet)
| @@ -29,8 +29,8 @@ const fetchWithCache = async ( | |||
| return cachedResponse | |||
| } | |||
| fetchOptions.headers.set('Content-Type', 'application/json') | |||
| const _fetch = timeout ? fetchWithTimeout({ timeout }) : window.fetch | |||
| const response = await _fetch(url, { | |||
| const fetchWithTimeout = getFetchWithTimeout(timeout) | |||
There was a problem hiding this comment.
Do we want to add a try/catch here?
rekmarks
force-pushed
the
fetch-with-timeout-everywhere
branch
from
35f589d to
e028f78
Compare
Builds ready [e028f78]
Page Load Metrics (596 ± 69 ms)
|
Uses
fetchWithTimeouteverywhere except indevelopment/files.fetch-with-timeout.jstoshared/modulesgetFetchWithTimeoutgetFetchWithTimeoutdefault to 30 seconds (that's the longest timeout used in practice)