-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[cli] Fix permission issue when user doesn't have permission to view app #25650
Conversation
Ping. This is a release blocker. |
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.
Approving to unblock the release but I think there is a chance we'll want to revisit this and make the error checking even more precise, specifically by checking the GraphQL error for a specific error code (and doing similar for CombinedErrors if there is just one underlying error and it's a GraphQL error that matches)
I thought about it, but there isn't a callsite in this codebase that does it with Graphql errors (only ApiV2Error and only for OTP). |
35f51a5
to
4447ed1
Compare
0efaa64
to
25fdcc8
Compare
Why
When #21989 was written, it forgot to account for cases where the app isn't visible to the viewer (happens when app privacy is set to private). It had incorrectly assumed that all app privacy delegated to the viewer's permissions on the owning account.
Closes ENG-10719.
How
The fix is to treat errors thrown when trying to query an app as permission errors and return null for
fetchAndCacheNewDevelopmentCodeSigningInfoAsync
. Note that this does mask network errors, I think this is ignored when offline anyways.Test Plan
Run new tests.
Manual test plan:
nexpo start
Checklist
npx expo prebuild
& EAS Build (eg: updated a module plugin).