facebook / hermes

A JavaScript engine optimized for running React Native.
https://hermesengine.dev/
MIT License
9.91k stars 641 forks source link

RN app crashes with javascript error when using Hermes #148

Closed compojoom closed 4 years ago

compojoom commented 5 years ago

We just ran into a strange crash in Production mode and after several hours of debugging it turned out that if we disable Hermes and use JSC the app works fine.

grafik

So basically this code causes the crash

export function* handleLoadAccount(
  action: ActionType<typeof accountActions.loadAccountRequest>
): Saga<void> {
  try {
    // xxxx
    const initialNavigationAction = yield call(checkForInitialURL)

    yield call(client.loadAccount, accountToLoad.walletCredentials)

    //xxxx
  } catch (error) {
    yield put(accountActions.loadAccountFail({ error }))
  } finally {
    yield put(toggleAppLoading(false))
  }
}

The error that is shown to us comes from this line: const initialNavigationAction = yield call(checkForInitialURL) If we remove it - then the app works fine. CheckForInitialURL does nothing special:

export async function checkForInitialURL() {
  const url = await Linking.getInitialURL()

// some more code that is not executed because url is null in this case
}

if I switch those 2 lines around:

    const initialNavigationAction = yield call(checkForInitialURL)

    yield call(client.loadAccount, accountToLoad.walletCredentials)

then the app works. As I said it works fine with JSC.

The app is currently in a private repo, but if soemeone from the team would like to give it a try I can provide you with access to it.

We have a standard build.gradle:

apply plugin: "com.android.application"
apply plugin: "io.fabric"

import com.android.build.OutputFile

/**
 * The react.gradle file registers a task for each build variant (e.g. bundleDebugJsAndAssets
 * and bundleReleaseJsAndAssets).
 * These basically call `react-native bundle` with the correct arguments during the Android build
 * cycle. By default, bundleDebugJsAndAssets is skipped, as in debug/dev mode we prefer to load the
 * bundle directly from the development server. Below you can see all the possible configurations
 * and their defaults. If you decide to add a configuration block, make sure to add it before the
 * `apply from: "../../node_modules/react-native/react.gradle"` line.
 *
 * project.ext.react = [
 *   // the name of the generated asset file containing your JS bundle
 *   bundleAssetName: "index.android.bundle",
 *
 *   // the entry file for bundle generation
 *   entryFile: "index.android.js",
 *
 *   // https://facebook.github.io/react-native/docs/performance#enable-the-ram-format
 *   bundleCommand: "ram-bundle",
 *
 *   // whether to bundle JS and assets in debug mode
 *   bundleInDebug: false,
 *
 *   // whether to bundle JS and assets in release mode
 *   bundleInRelease: true,
 *
 *   // whether to bundle JS and assets in another build variant (if configured).
 *   // See http://tools.android.com/tech-docs/new-build-system/user-guide#TOC-Build-Variants
 *   // The configuration property can be in the following formats
 *   //         'bundleIn${productFlavor}${buildType}'
 *   //         'bundleIn${buildType}'
 *   // bundleInFreeDebug: true,
 *   // bundleInPaidRelease: true,
 *   // bundleInBeta: true,
 *
 *   // whether to disable dev mode in custom build variants (by default only disabled in release)
 *   // for example: to disable dev mode in the staging build type (if configured)
 *   devDisabledInStaging: true,
 *   // The configuration property can be in the following formats
 *   //         'devDisabledIn${productFlavor}${buildType}'
 *   //         'devDisabledIn${buildType}'
 *
 *   // the root of your project, i.e. where "package.json" lives
 *   root: "../../",
 *
 *   // where to put the JS bundle asset in debug mode
 *   jsBundleDirDebug: "$buildDir/intermediates/assets/debug",
 *
 *   // where to put the JS bundle asset in release mode
 *   jsBundleDirRelease: "$buildDir/intermediates/assets/release",
 *
 *   // where to put drawable resources / React Native assets, e.g. the ones you use via
 *   // require('./image.png')), in debug mode
 *   resourcesDirDebug: "$buildDir/intermediates/res/merged/debug",
 *
 *   // where to put drawable resources / React Native assets, e.g. the ones you use via
 *   // require('./image.png')), in release mode
 *   resourcesDirRelease: "$buildDir/intermediates/res/merged/release",
 *
 *   // by default the gradle tasks are skipped if none of the JS files or assets change; this means
 *   // that we don't look at files in android/ or ios/ to determine whether the tasks are up to
 *   // date; if you have any other folders that you want to ignore for performance reasons (gradle
 *   // indexes the entire tree), add them here. Alternatively, if you have JS files in android/
 *   // for example, you might want to remove it from here.
 *   inputExcludes: ["android/**", "ios/**"],
 *
 *   // override which node gets called and with what additional arguments
 *   nodeExecutableAndArgs: ["node"],
 *
 *   // supply additional arguments to the packager
 *   extraPackagerArgs: []
 * ]
 */

project.ext.react = [
        entryFile            : "index.js",
        bundleInDebug        : false,
        bundleInDevelop      : true,
        bundleInStaging      : true,
        bundleInRelease      : true,
        jsBundleDirDebug     : "$buildDir/intermediates/assets/debug",
        jsBundleDirDevelop   : "$buildDir/intermediates/assets/develop",
        jsBundleDirStaging   : "$buildDir/intermediates/assets/staging",
        jsBundleDirRelease   : "$buildDir/intermediates/assets/release",
        resourcesDirDebug    : "$buildDir/intermediates/res/merged/debug",
        resourcesDirDevelop  : "$buildDir/intermediates/res/merged/develop",
        resourcesDirStaging  : "$buildDir/intermediates/res/merged/staging",
        resourcesDirRelease  : "$buildDir/intermediates/res/merged/release",
        nodeExecutableAndArgs: hasProperty('NODE_PATH') ? [NODE_PATH] : ["node"],
        devDisabledInDebug   : false,
        devDisabledInDevelop : true,
        devDisabledInStaging : true,
        devDisabledInRelease : true,
        inputExcludes        : ["ios/**", "__tests__/**"],
        extraPackagerArgs: [ '--minify=false' ],
        enableHermes: true,
]

apply from: "../../node_modules/react-native/react.gradle"
apply from: "../../node_modules/react-native-code-push/android/codepush.gradle"
apply from: "../../node_modules/react-native-vector-icons/fonts.gradle"

/**
 * Set this to true to create two separate APKs instead of one:
 *   - An APK that only works on ARM devices
 *   - An APK that only works on x86 devices
 * The advantage is the size of the APK is reduced by about 4MB.
 * Upload all the APKs to the Play Store and people will download
 * the correct one based on the CPU architecture of their device.
 */
def enableSeparateBuildPerCPUArchitecture = false

/**
 * Run Proguard to shrink the Java bytecode in release builds.
 */
def enableProguardInReleaseBuilds = false

/**
 * The preferred build flavor of JavaScriptCore.
 *
 * For example, to use the international variant, you can use:
 * `def jscFlavor = 'org.webkit:android-jsc-intl:+'`
 *
 * The international variant includes ICU i18n library and necessary data
 * allowing to use e.g. `Date.toLocaleString` and `String.localeCompare` that
 * give correct results when using with locales other than en-US.  Note that
 * this variant is about 6MiB larger per architecture than default.
 */
def jscFlavor = 'org.webkit:android-jsc:+'
/**
 * Whether to enable the Hermes VM.
 *
 * This should be set on project.ext.react and mirrored here.  If it is not set
 * on project.ext.react, JavaScript will not be compiled to Hermes Bytecode
 * and the benefits of using Hermes will therefore be sharply reduced.
 */
def enableHermes = project.ext.react.get("enableHermes", false);

def getTimestamp() {
    Long tsLong = System.currentTimeMillis() / 1000L;
    def ts = (int) tsLong.toInteger();
    return ts
}

android {
    compileSdkVersion rootProject.ext.compileSdkVersion

    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }

    defaultConfig {
        applicationId "mobileapp"
        minSdkVersion rootProject.ext.minSdkVersion
        targetSdkVersion rootProject.ext.targetSdkVersion
        versionCode getTimestamp()
        versionName "${versionMajor}.${versionMinor}.${versionPatch}${versionPretag}"
        missingDimensionStrategy "react-native-camera", "general"
        multiDexEnabled true
    }

    signingConfigs {
        develop {
            if (project.hasProperty('MYAPP_RELEASE_STORE_FILE')) {
                storeFile file(MYAPP_RELEASE_STORE_FILE)
                storePassword MYAPP_RELEASE_STORE_PASSWORD
                keyAlias MYAPP_RELEASE_KEY_ALIAS
                keyPassword MYAPP_RELEASE_KEY_PASSWORD
            }
        }
        staging {
            if (project.hasProperty('MYAPP_RELEASE_STORE_FILE')) {
                storeFile file(MYAPP_RELEASE_STORE_FILE)
                storePassword MYAPP_RELEASE_STORE_PASSWORD
                keyAlias MYAPP_RELEASE_KEY_ALIAS
                keyPassword MYAPP_RELEASE_KEY_PASSWORD
            }
        }
        release {
            if (project.hasProperty('MYAPP_RELEASE_STORE_FILE')) {
                storeFile file(MYAPP_RELEASE_STORE_FILE)
                storePassword MYAPP_RELEASE_STORE_PASSWORD
                keyAlias MYAPP_RELEASE_KEY_ALIAS
                keyPassword MYAPP_RELEASE_KEY_PASSWORD
            }
        }
    }
    splits {
        abi {
            reset()
            enable enableSeparateBuildPerCPUArchitecture
            universalApk false  // If true, also generate a universal APK
            include "armeabi-v7a", "x86", "arm64-v8a", "x86_64"
        }
    }
    buildTypes {
        debug {
            applicationIdSuffix ".debug"
        }

//        develop {
//            minifyEnabled enableProguardInReleaseBuilds
//            proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
//            signingConfig signingConfigs.develop
//            applicationIdSuffix ".develop"
//            matchingFallbacks = ['release', 'debug']
//        }
//
//        staging {
//            minifyEnabled enableProguardInReleaseBuilds
//            proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
//            signingConfig signingConfigs.staging
//            applicationIdSuffix ".staging"
//            matchingFallbacks = ['release', 'debug']
//        }

        release {
            minifyEnabled enableProguardInReleaseBuilds
            proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
            signingConfig signingConfigs.release
        }
    }
    flavorDimensions "version"
    productFlavors {
        develop {
            applicationId "mobileapp.develop"
        }

        staging {
            applicationId "mobileapp.staging"
        }

        production {
            applicationId "mobileapp"
        }
    }

    // applicationVariants are e.g. debug, release
    applicationVariants.all { variant ->
        variant.outputs.each { output ->
            // For each separate APK per architecture, set a unique version code as described here:
            // https://developer.android.com/studio/build/configure-apk-splits.html
            def versionCodes = ["armeabi-v7a": 1, "x86": 2, "arm64-v8a": 3, "x86_64": 4]
            def abi = output.getFilter(OutputFile.ABI)
            if (abi != null) {  // null for the universal-debug, universal-release variants
                output.versionCodeOverride =
                        versionCodes.get(abi) * 1048576 + defaultConfig.versionCode
            }
        }
    }

    dexOptions {
        jumboMode true
    }
}

dependencies {
    implementation fileTree(dir: "libs", include: ["*.jar"])

    // Firebase dependencies
    implementation "com.google.firebase:firebase-core:17.2.0"
    implementation "com.google.firebase:firebase-analytics:17.2.0"
    implementation "com.google.firebase:firebase-messaging:20.0.0"
    implementation('com.crashlytics.sdk.android:crashlytics:2.9.9@aar') {
        transitive = true
    }

    implementation "com.facebook.react:react-native:+"  // From node_modules

    if (enableHermes) {
        def hermesPath = "../../node_modules/hermes-engine/android/";
        debugImplementation files(hermesPath + "hermes-debug.aar")
        releaseImplementation files(hermesPath + "hermes-release.aar")
    } else {
        implementation jscFlavor
    }
}

// Run this once to be able to run the application with BUCK
// puts all compile dependencies into folder libs for BUCK to use
task copyDownloadableDepsToLibs(type: Copy) {
    from configurations.compile
    into 'libs'
}

apply from: file("../../node_modules/@react-native-community/cli-platform-android/native_modules.gradle"); applyNativeModulesAppBuildGradle(project)
apply plugin: 'com.google.gms.google-services'

// go around a google play services mismatch
com.google.gms.googleservices.GoogleServicesPlugin.config.disableVersionCheck = true

Tested on RN 0.61.4 with hermes 0.2.1

avp commented 5 years ago

If you're using the React Native JS bundler, then the code you're writing gets run through Babel before it gets sent to Hermes. That would likely greatly transform the generator and the async function into something completely different using regenerator, at the moment.

To help reproduce this, it would be useful to have a minimal JS bundle which demonstrates the error when run with Hermes. For example, a file bundle.js which when run with Hermes using ./hermes bundle.js throws the error.

Alternately, you can put a small React Native project on GitHub which shows the issue when running Hermes on your Android phone and we can try and reproduce from there. It would be much harder for us to diagnose an issue on the full size app in a private repository because there would simply be significantly more code compared to a minimal example which shows the error. In particular, it seems that the error is being thrown by either your app or a third-party library that wants a wallet to be loaded, so figuring out why it doesn't think the wallet is loaded is a good first step to finding a minimal failing case.

compojoom commented 5 years ago

@avp - yes, the error is thrown by our library, but only when ran on Hermes and not on JSC

I'll try to make a smaller example that reproduces the error, but I'm not really very optimistic that I'll manage to reproduce it outside of the spaghetti code we currently have :D

avp commented 4 years ago

Closing this for now, feel free to reopen if you are still able to reproduce independently.