1206 error responses occasionally being cached by cached utility (Revert Reversion) #1376
Conversation
|
🚅 Deployed to the app-pr-1376 environment in Drips App
|
src/routes/api/tlv/+server.ts
Outdated
| const errorContent = await driptsTokenHoldingRes.text(); | ||
| // eslint-disable-next-line no-console | ||
| console.error('Etherscan returned error response', errorContent); | ||
| return new Response('[]', { headers: { 'Content-Type': 'application/json' } }); |
There was a problem hiding this comment.
Why return an empty array? it usually returns a number as a string. on the actual default-explore-page.svelte it checks for typeof tlv === 'number'. I think ideally:
- this endpoint returns either nothing or an error 500 with empty body in case there's an etherscan error
- the logic calling in the TLV value catches an error response from this endpoint, and returns
nullin that case default-explore-page.svelteexplicitly accepts eithernumberornullastlvprop, and conditionally renders the TLVif tlv !== null
wdyt?
There was a problem hiding this comment.
Yes down to change! I was following the logic in the previous lines that return an empty array if there's no etherscan key. It looks like it eventually ends up in a Intl formatting method which returns $0.0 when given an empty array...weird!
I think returning null is great in these cases. If the tlv endpoint returns a 500, the whole loading blows up as it is currently.
There was a problem hiding this comment.
I see, didn't catch that! Yeah, if there's no etherscanApiKey i think it also should be returning null and handle it the same way as when there's an error — produce a server-side log, and gracefully hide the TLV card.
There was a problem hiding this comment.
Yeah that makes total sense, will do
…hed util…" This reverts commit 5365d24.
* Handle those errors at a higher level
mhgbrown
force-pushed
the
revert-1372-revert-1363-mhgbrown-1206-error-responses-occasionally-being-cached-by-cached-utility
branch
from
d6e86d4 to
a7e3679
Compare
mhgbrown
deleted the
revert-1372-revert-1363-mhgbrown-1206-error-responses-occasionally-being-cached-by-cached-utility
branch
This PR attempts to limit the incidence of caching non-OK responses via the
cachedutility andfetch. Getting blog posts is method-ized and reused in places where code was being duplicated. I wanted to do the same with theapi/tlvendpoint (and re-usegetCmcPrices!), but didn't feel comfortable going all in without a working Etherscan api connection.I also ran
npm run format, which re-formatted src/lib/utils/puppeteer.ts...I was just working on that so it's surprising to me that it needed re-formatting ¯\(ツ)/¯Reverts and builds upon #1372