expo / custom-expo-updates-server

238 stars 67 forks source link

Missing expo config in Manifest Response Body #12

Closed SrZorro closed 1 year ago

SrZorro commented 1 year ago

Starting problem

Assets like images fail to load or the app crashes after an update using the example server (./expo-updates-server) when using method Linking.createURL(path, namedParameters) of expo-linking

adb logcat shows the following error when assets fail to load:

ReactNativeJS: Error: Cannot make a deep link into a standalone app with no custom scheme defined

And when it crashes:

ReactNativeJS: Error: Cannot make a deep link into a standalone app with no custom scheme defined
--------- beginning of crash
AndroidRuntime: FATAL EXCEPTION: expo-updates-error-recovery
AndroidRuntime: Process: <redacted>, PID: 4427
AndroidRuntime: com.facebook.react.common.JavascriptException: Error: Cannot make a deep link into a standalone app with no custom scheme defined, stack:

Cause

expo-linking requires the field (app.json|app.config.json).expo.scheme to be set, but following the code from that error in expo-linking backtracking to line 120, calls the method collectManifestSchemes that makes multiple gets to Constants.expoConfig (lines 68, 69, 73, 76), someway this method fails to return the manifests and returns an empty array, causing the throw error

Making another standalone build, commenting the call to Linking.createURL and with console.log("Constants.expoConfig::", Constants.expoConfig); in App.tsx

If we launch the app, without a new OTA update, console.log returns the content from app.json|app.config.json (expo config going forward) that is bundled with the app

If we launch the app, WITH a new OTA update, console.log returns an object with the exact response from the manifest endpoint aka the Manifest Response Body, but Constants.expoConfig it is expected to have expo config as stated in constants documentation

Real problem

Documentation or a fix is missing for how we should add the expo config to the metadata response, as the current implementation anything that expects that Constants.expoConfig to be expo config, will receive a Manifest Response body instead, in my case breaking expo-linking and showing the version of the app as undefined when getting Constants.expoConfig?.version

EAS Updates implementation

As stated from this pull request to eas-cli, referencing the pull request /expo/universe/pull/7893 the EAS Updates server (private repo I guess? Returns 404) will support storing and serving the extra.expoClient field in the eas update manifest

So when using eas publish, it's adding to the metadata that will upload to EAS Updates the expo config in the extra.expoClient field

But this special extra key (expoClient) is not documented in the Manifest Response Body as it only says: extra: { [key: string]: any }; about the extra field

Hacks / Workarounds

These are two ways I got Constants.expoConfig to have the expected fields from expo config after an OTA Update

Append expo config to metadata response

Appending to ./expo-updates-server/pages/api/manifest.ts:47 manifest, the expo config, Constants.expoConfig will have a union of expo config & Manifest Response Body

This was the first hack I got working to make the app get an update and not break expo-linking & Constants.expoConfig?.version

EAS Updates current implementation?

I did all my investigations without checking a real world response from EAS Updates, but making a puzzle with all the source code available from expo & eas I think I understand how the EAS Update service works to get the expo config in an OTA update and have it work

Checking the getter for Constants.expoConfig

It calls getManifest that if all the checks are OK, just returns rawManifest

rawManifest in a nutshell will be: if we got an OTA update, it will be Manifest Response Body, if not, the bundled expo config (from ExponentConstants.manifest)

This is where all my problems come from, this rawManifest that can be Manifest Response Body or expo config, but by default in an OTA Update will be Manifest Response Body if we don't make the specific "dance" to get the expo config from the Manifest Response Body

One would think that by default, if no expo config is given in the OTA Update (something that is not documented, or I couldn't find after almost a week of having fun with this), it will fall back to the bundled one, but instead just sets Constants.expoConfig to Manifest Response Body, what a rabbit hole I had to go to find out why our app wasn't working because expo config wasn't there after an OTA Update

The dance

Checking the code from the EAS Update implementation, is when I found out that extra.expoClient IS expo config, so I could finish the puzzle from the expoConfig getter

So, to make the metadata endpoint return the expo config, and get Constants.expoConfig to return it, I had to:

Change the ./expo-updates-server/pages/api/manifest.ts:47 manifest object type with the following modifications:

import { ExpoConfig } from https://github.com/expo/expo/blob/main/packages/%40expo/config-types/src/ExpoConfig.ts
type Manifest = {
  id: string;
  createdAt: string;
  runtimeVersion: string;
  launchAsset: Asset;
  assets: Asset[];
  metadata: { 
    [key: string]: string;
    _randomKey: "random Value";
  };
  extra: { 
    [key: string]: any;
    expoClient: ExpoConfig;
  };
}

Constants.expoConfig getter will check if rawManifest has a metadata key (metadata._randomKey in the modified type to ensure we have metadata). If we have metadata, it will return rawManifest.extra?.expoClient ?? null

Because we set expoClient in the type example, we will finally get Constants.expoConfig with the expected value of expo config from the response of Manifest Response Body. Hurray 🎉

The Manifest Response body spec says "The metadata associated with an update. It is a string-valued dictionary. The server MAY send back anything it wishes to be used for filtering the updates. The metadata MUST pass the filter defined in the accompanying expo-manifest-filters header." but I didn't understand how it's supposed to be used, that's why I added the _randomKey

What I wish

This repository guide about how to make an update, in a nutshell the steps are:

  1. Make modifications to app
  2. expo export --experimental-bundle
  3. Copy dist to expo-update-server/updates/<runtime version>

IMHO I think that the expo config should be as easy to update, as the exported bundle

expo export --experimental-bundle could also export the expo config in a file called app.config.json next to metadata.json in the dist directory

And then the example server in expo-updates-server implement the needed logic to add it to extra.expoClient & add an empty metadata key (or whatever should have)

wschurman commented 1 year ago

@SrZorro - excellent write up!

So there's a few issues in here:

  1. This repo (custom-expo-updates-server) incorrectly implements the expo-updates specification. Namely, the metadata field is required in the response (as you noticed). The fix is to add metadata: {} to the update response, since it is required for determining manifest type in expo-constants (JS) and expo-manifests (native code). I will put up a PR for this. I'm honestly shocked expo-manifests is working without it so an investigation into why it works without it will be the first step.
  2. We need to add information about the extra field in the expo-updates protocol. expo-updates itself shouldn't specify what fields are in the extra field, but it should specify where and how extra fields work. I'll put up a separate PR for this.
  3. We need to add documentation to libraries that make use of expoConfig indicating which config fields they depend on. This is a much more difficult problem to address since there's so many libraries, but starting with expo-linking seems good. I'll put up a PR for this as well.

This example repo is designed to be a barebones implementation of the expo-updates protocol specification: https://docs.expo.dev/technical-specs/expo-updates-0/. But that doesn't mean that it shouldn't be easy to fork it and add additional expo libraries to it, so I think that is the end goal.

wschurman commented 1 year ago

@SrZorro - I implemented the first two items I proposed. The last one is a bit tough to do thoroughly since so many other libraries require config.

What I wish This repository guide about how to make an update, in a nutshell the steps are:

  1. Make modifications to app
  2. expo export --experimental-bundle
  3. Copy dist to expo-update-server/updates/<runtime version>

IMHO I think that the expo config should be as easy to update, as the exported bundle

expo export --experimental-bundle could also export the expo config in a file called app.config.json next to metadata.json in the dist directory

And then the example server in expo-updates-server implement the needed logic to add it to extra.expoClient & add an empty metadata key (or whatever should have)

This is an interesting idea. I'm not sure I'm the right person to assess whether this should be part of the expo export command though versus just having it be an extra command. Maybe @byCedric or @EvanBacon can comment. Something like adding a flag: expo export --expo-config.

The same thing can be done by just using getConfig from @expo/config. If we think about how EAS does it, it just reads the config and uploads it to the server. So the equivalent for this repo is to just have another step in the yarn expo-publish script that exports the result of getConfig into the dist directory as well, and then have the server read it and serve it.

wschurman commented 1 year ago

Alright, I think I'll update this repo to embed the config in the manifest as a demonstration.