App crashes after updating from Relay 7 to Relay 11 with undefined operation

[high-performance-hery]

After upgrading from Relay 7 to Relay 11, we've been having a crash in our app that's quite hard to reproduce.

It seems to happen when we open from the background using a useQuery hook with fetch policy 'store-then-network', so it must be booting from the offline store. Previously we were using a QueryRenderer.

We upgraded from those versions:

    "react-relay": "~7.1.0",
    "react-relay-network-modern": "~4.7.7",
    "react-relay-offline": "~1.1.0",
    "relay-compiler": "~7.1.0",
    "relay-compiler-language-typescript": "~10.1.3",

to the following versions

    "react-relay": "11.0.2",
    "react-relay-network-modern": "~4.7.7",
    "react-relay-offline": "4.0.0",
    "relay-compiler": "11.0.2",
    "relay-compiler-language-typescript": "~14.1.1",
    "@types/relay-runtime": "12.0.0"

Remote Sentry stack trace

Local stack trace

[morrys]

Hi @high-performance-hery,
to solve your problem you need to migrate the data present in the store as reported in the release note:
https://github.com/morrys/wora/releases/tag/relay-store%404.0.0

[high-performance-hery]

Hi @morrys, that makes sense, think we might've missed that. Thank you!

[high-performance-hery]

@morrys We've implemented the store migration as follow but the crash still occurs.
Do you think it could be caused by the possibly undefined restoredState?

const persistOptionsRecords: CacheOptions = {
    // Add store migration step from https://github.com/morrys/wora/releases/tag/relay-store%404.0.0
    mergeState: (restoredState, _) => {
        // We need to add this line to make TS happy since restoredState is optional
        // I believe ignoring an undefined restoredState is the correct behaviour?
        if (!restoredState) return {}
        return Object.keys(restoredState).reduce((acc, key) => {
            const previous = acc[key]
            if (previous.selector) {
                acc[key] = {
                    ...previous,
                    operation: {
                        root: previous.selector
                    },
                    dispose: true,
                    refCount: 0,
                    fetchTime: previous.retainTime,
                    ttl: 1
                }
            }
            return acc
        }, restoredState)
    }
}

[morrys]

Do you have the same error or a different one?

Does the application use server side rendering?

The default merge function is this here:
https://github.com/morrys/wora/blob/master/packages/cache-persist/src/Cache.ts#L21
(restoredState, initialState): DataCache => (restoredState? restoredState: initialState);

The merge function has two parameters, restoredStare & initialState
initialState is set when the store is populated before the restore (usually in SSR)
restoredState is the state retrieved from the store

In case initialState is populated, the merge function should return the merge between restoredState & initialState

If both are empty, you have nothing to migrate and you can return {}

[high-performance-hery]

Hi @morrys ..sorry for the delay here. I’ve only just got round to being able to recreate the bug.
We don’t use SSR, this is a React Native app with the state stored in Async Storage. If I log from the mergeState function above, we do receive a simple initialState object:

Does this make sense in our context?
We are still getting the same bug. I can recreate as follows:

• Run the app on v7 of Relay. Press on a few screens to persist the responses to async storage.
• Checkout the upgraded app and rebuild. Set the ttl in CacheOptions to a small number.
• Open the app. Put to the background for a time longer than the TTL.
• Open a screen in the app that we persisted. Press back. The app crashes with the previous error

Our merge function is this one.

I tried merging initialState and restoredState as follows, but I have the same issue:

mergeState: (restoredState, _initialState) => {
    // We need to add this line to make TS happy since restoredState is optional
    // I believe ignoring an undefined restoredState is the correct behaviour?

    if (!restoredState) return {}
    const updatedRestoredState = Object.keys(restoredState).reduce((acc, key) => {
        const previous = acc[key]
        if (previous.selector) {
            acc[key] = {
                ...previous,
                operation: {
                    root: previous.selector
                },
                dispose: true,
                refCount: 0,
                fetchTime: previous.retainTime,
                ttl: 1
            }
        }
        return acc
    }, restoredState)

    if (_initialState) {
        return {
            ..._initialState,
            ...updatedRestoredState
        }
    }

    return updatedRestoredState
}

[morrys]

Can you change this piece of code in node_modules like this below and send me this information in a readable way?

     console.log("this._roots.values()", this._roots.values()); 
     for (const rootValue of this._roots.values()) {
        console.log("rootValue", rootValue);
        const {operation} = rootValue;
        const selector = operation.root;
        RelayReferenceMarker.mark(

Thank you

[morrys]

can you send me the piece of code where you create the environment, store and record source?

[high-performance-hery]

Actually one question while I grab that information - I have tried to follow the setup here.

Should we add mergeState to persistOptionsStore or persistOptionsRecords?

We currently have it in persistOptionsRecords, which might explain the problem if that’s incorrect.

[morrys]

https://github.com/morrys/wora/blob/master/packages/relay-store/__tests__/Store-persisted-test.ts#L101
to persistOptionsStore 👍