-
Notifications
You must be signed in to change notification settings - Fork 224
Fix token refresh for Business Platform API #6490
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
|
/snapit |
Coverage report
Show files with reduced coverage 🔻
Test suite run success3237 tests passing in 1337 suites. Report generated by 🧪jest coverage report action from 3e52bb5 |
|
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20251007130258Caution After installing, validate the version by running just |
This comment has been minimized.
This comment has been minimized.
d648bde to
3e52bb5
Compare
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/21223
We keep seeing some unhandled 401 errors like this, that shouldn't be happening.
When the CLI gets a 401, it tries to get a new token (by refreshing or starting the full authentication flow) and then retries the request.
The problem is that the retry after the refresh process was always using the App Management API token, but if the query was against Business Platform API, then it was obviously getting another 401, that was reported.
WHAT is this pull request doing?
Pass the API type to the refresh function, so that it returns the proper token for each one.
How to test your changes?
The token must become invalid right before a Business Platform request, so it's not easy to reproduce. I faked it this way:
p shopify app devappContextResult.developerPlatformClient.session.businessPlatformTokentowrongMeasuring impact
How do we know this change was effective? Please choose one:
Checklist