Skip to content

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

Merged
merged 5 commits into from
Mar 28, 2025
Merged

fix: retry IPNI queries on 5xx errors #126

merged 5 commits into from
Mar 28, 2025

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Mar 27, 2025

  • refactor: add assert-ok-response from npm
  • deps: add pRetry from npm
  • chore: fix errors reported by npx standard
  • fix: retry IPNI requests on 5xx errors

Links:

bajtos added 4 commits March 27, 2025 11:05
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>
@bajtos bajtos requested review from pyropy and NikolasHaimerl March 27, 2025 10:14
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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also above

Copy link
Member

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(() => {
Copy link
Member

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!

Copy link
Member Author

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 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos bajtos merged commit d48de63 into main Mar 28, 2025
2 checks passed
@bajtos bajtos deleted the retry-ipni-5xx branch March 28, 2025 07:46
@github-project-automation github-project-automation bot moved this to ✅ done in Space Meridian Mar 28, 2025
Copy link

sentry-io bot commented Apr 9, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ExecaError: Command was killed with SIGABRT (Aborted): /usr/src/app/modules/zinnia/zinniad spark/main.js getRetrievalProviders(ipni-client) View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ done
Development

Successfully merging this pull request may close these issues.

2 participants