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

[ios] show helpful error message when expo-cli responds with an incompatible project #10508

Merged
merged 2 commits into from
Oct 2, 2020

Conversation

esamelson
Copy link
Contributor

@esamelson esamelson commented Oct 2, 2020

Why

fix for #10344

When serving projects in development, expo-cli does not respect the SDK version headers in manifest requests and will respond with the project's manifest rather than an error code if the SDK version is incompatible.

Pre-expo-updates, we did our own client-side compatibility check when loading manifests for this reason, and spoofed the www error code response for manifests that were incompatible. With expo-updates, we do the client-side compatibility check at the time of launching an update from the database, which is why this has resulted in the less-helpful error message of "No launchable update found in database."

How

Add a separate call to the verifyManifestSdkVersion: method when loading a new remote update. If the update is incompatible, we abort and show an error.

This does add a tiny bit more stateful behavior to EXAppLoaderExpoUpdates, so it would be good to do the follow up steps in the future to add some more robust handling and remove the need for this statefulness.

Test Plan

  • load SDK 35 project from expo-cli, see correct error messgae
  • load SDK 50 project from expo-cli, see correct error message
  • load SDK 35 project from www, see correct error message
  • load SDK 39 project from expo-cli, works
  • load SDK 39 project from www, works

Follow up

  • apply same fix on Android
  • add more robust logic in expo-updates to ensure that an update is compatible before downloading it
  • follow up to determine if this behavior in expo-cli is really correct, or if we want to respond with an error message/code if the client's request suggests it's incompatible with the JS app

Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

i agree that longer term changing expo-cli to handle this better would be good 👍

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

Successfully merging this pull request may close these issues.

2 participants