Conversation
Builds ready [35f589d]
Page Load Metrics (534 ± 43 ms)
|
darkwing
left a comment
There was a problem hiding this comment.
Marking as request changes, if only to see the comments.
I also wonder if we should up the 30 second; I wish we had metrics. We can probably start with 30 and if we get timeout reports, we can adjust.
| 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)
| 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?
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)