Skip to content
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

Replace got in update-check to prevent CLI crash #20693

Merged
merged 2 commits into from
Dec 10, 2023

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented Dec 10, 2023

Scope

What's changed:

  • There's an issue in got when previously cached response gets revalidated: Hangs on revalidated response from cache sindresorhus/got#2317
  • The update-check which is performed when spawning the Directus CLI uses got to fetch the information from the NPM registry and caches the response
  • When the Directus CLI is then called again after > 5min (expiration of cached NPM response), got correctly revalidates the response but then hangs indefinitely which is detected by Node.js and as a consequence Node.js terminates the process (exit code 13, unfinished top-level await)
  • We can see that happening in Blackbox Tests:
  • To fix that issue we replace got with axios in combination with axios-cache-interceptor

Potential Risks / Drawbacks

I've successfully tested various scenarios, so this PR comes with minimal risk:

  • First request without cache
  • Subsequent request with cached response
  • Revalidation of cached response after > 5min
  • Offline mode

Review Notes / Questions

None

Copy link

changeset-bot bot commented Dec 10, 2023

🦋 Changeset detected

Latest commit: 76bbe93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@directus/update-check Patch
directus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@paescuj paescuj changed the title Replace got in update-check to prevent crash with cached response Replace got in update-check to prevent CLI crash Dec 10, 2023
@paescuj paescuj self-assigned this Dec 10, 2023
@paescuj paescuj merged commit 0587948 into main Dec 10, 2023
4 checks passed
@paescuj paescuj deleted the fix-update-check-cache-crash branch December 10, 2023 13:45
@github-actions github-actions bot added this to the Next Release milestone Dec 10, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

1 participant