-
Notifications
You must be signed in to change notification settings - Fork 4
fix: retry IPNI queries on 5xx errors #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
console.log('IPNI returned %s provider results', providerResults.length) | ||
} catch (err) { | ||
console.error('IPNI query failed.', err) | ||
return { indexerResult: 'ERROR_FETCH' } | ||
return { | ||
indexerResult: typeof err.statusCode === 'number' ? `ERROR_${err.statusCode}` : 'ERROR_FETCH' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indexerResult: typeof err.statusCode === 'number' ? `ERROR_${err.statusCode}` : 'ERROR_FETCH' | |
indexerResult: typeof err.status === 'number' ? `ERROR_${err.status}` : 'ERROR_FETCH' |
https://developer.mozilla.org/en-US/docs/Web/API/Response/status
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, this is the error from assertOkResponse
, not fetch
. Nevermind
@@ -100,7 +99,8 @@ export default class Spark { | |||
controller.abort() | |||
}, 60_000) | |||
} | |||
maxDurationTimeout = setTimeout(() => { | |||
|
|||
const maxDurationTimeout = setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this clean up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are welcome. It was reported by npx standard
. I am surprised this passed our CI checks 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
npx standard
Links: