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

throw if getState, subscribe, or unsubscribe called while dispatching #1569

Merged
merged 5 commits into from Jul 19, 2016

Conversation

5 participants
@mjw56
Copy link
Contributor

mjw56 commented Apr 2, 2016

This fixes #1568.

@mjw56

This comment has been minimized.

Copy link
Contributor Author

mjw56 commented Apr 2, 2016

Hey @gaearon, I added checks and updated tests. Let me know your thoughts, thanks!

var isSubscribed = true

ensureCanMutateNextListeners()
nextListeners.push(listener)

return function unsubscribe() {
return function unsubscribe() {

This comment has been minimized.

@gaearon

gaearon Apr 2, 2016

Contributor

Nit: empty spaces

@@ -71,6 +71,10 @@ export default function createStore(reducer, initialState, enhancer) {
* @returns {any} The current state tree of your application.
*/
function getState() {
if (isDispatching) {
throw new Error('Reducers may not access store state.')

This comment has been minimized.

@gaearon

gaearon Apr 2, 2016

Contributor

Maybe change it to something a little more helpful? I’m up for more descriptive error messages. e.g. You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument, so pass it down from the top reducer instead of reading it from the store..

This comment has been minimized.

@mjw56

mjw56 Apr 2, 2016

Author Contributor

Yeah, I was just going with similar format to dispatch message. I like the more descriptive messages though. How about for subscribe and unsubscribe?

This comment has been minimized.

@mjw56

mjw56 Apr 2, 2016

Author Contributor

updated in f957b9f

@@ -71,6 +71,14 @@ export default function createStore(reducer, initialState, enhancer) {
* @returns {any} The current state tree of your application.
*/
function getState() {
if (isDispatching) {
throw new Error(
'You may not call store.getState() while the reducer is executing.' +

This comment has been minimized.

@gaearon

gaearon Apr 2, 2016

Contributor

These should have spaces right after the period, or you’ll end up with executing.The reducer

This comment has been minimized.

@mjw56

mjw56 Apr 2, 2016

Author Contributor

fixed with efd0d2e

@mjw56

This comment has been minimized.

Copy link
Contributor Author

mjw56 commented Apr 3, 2016

The subscribe and unsubscribe messages could probably be more helpful too.

I looked around, but didn't see anything specifically mentioning these scenarios in the documentation. Do you think this should be documented too?

Here is what I have for subscribe and unsubscribe:

// subscribe

'You may not call store.subscribe() while the reducer is executing. ' +
'If you would like to be notified after the store has been updated, subscribe from a ' +
'component and invoke store.getState() in the callback to access the latest state. ' +
'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'

// unsubscribe

'You may not unsubscribe from a store listener while the reducer is executing. ' +
'See http://redux.js.org/docs/api/Store.html#subscribe for more details.'
@gaearon

This comment has been minimized.

Copy link
Contributor

gaearon commented Apr 4, 2016

These sound good.

@gaearon gaearon added this to the 4.0 milestone Apr 4, 2016

@timdorr timdorr merged commit cb5a00b into reduxjs:master Jul 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mjw56 mjw56 deleted the mjw56:prevent-reducer-calls branch Jul 19, 2016

@gaearon

This comment has been minimized.

Copy link
Contributor

gaearon commented Aug 7, 2016

Note: it’s a breaking change. Since we merged this to master, we can’t release patches now.

@gaearon

This comment has been minimized.

Copy link
Contributor

gaearon commented Aug 7, 2016

I’m going to revert because we might need to release more patches.

I don’t currently have time to work on 4.x but if somebody wants to lead the effort I’m happy to discuss what should go there.

@timdorr

This comment has been minimized.

Copy link
Member

timdorr commented Aug 7, 2016

I can extract the commit back out to next, if you want.

@gaearon

This comment has been minimized.

Copy link
Contributor

gaearon commented Aug 7, 2016

👍

timdorr added a commit that referenced this pull request Aug 7, 2016

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages
@timdorr

This comment has been minimized.

Copy link
Member

timdorr commented Aug 7, 2016

Done. bb9b86c

timdorr added a commit that referenced this pull request Jan 5, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Apr 25, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Oct 22, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Oct 22, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Oct 23, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Nov 3, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

timdorr added a commit that referenced this pull request Nov 16, 2017

throw if getState, subscribe, or unsubscribe called while dispatching (
…#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

seantcoyote added a commit to seantcoyote/redux that referenced this pull request Jan 14, 2018

throw if getState, subscribe, or unsubscribe called while dispatching (
…reduxjs#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

seantcoyote added a commit to seantcoyote/redux that referenced this pull request Jan 14, 2018

throw if getState, subscribe, or unsubscribe called while dispatching (
…reduxjs#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages

tomipaul added a commit to tomipaul/redux that referenced this pull request Apr 8, 2018

throw if getState, subscribe, or unsubscribe called while dispatching (
…reduxjs#1569)

* throw error if getState, subscribe, or unsubscribe called while dispatching

* prevent throwing if not subscribed

* update getState error message

* fix space after period

* update subscribe/unsubscribe error messages
@phips28

This comment has been minimized.

Copy link

phips28 commented Sep 13, 2018

I came across this PR while researching the exception You may not unsubscribe from a store listener while the reducer is executing.
Its a React Native app.
I never use subscribe/unsubscribe in my own code, so maybe a library?
It only happens on Android releases, cant reproduce it in dev mode.
Root(native) is the class my index.android.js file, but there is no unsubscribe().
Can someone help me how to debug this?

Fatal Exception: com.facebook.react.common.JavascriptException: Error: You may not unsubscribe from a store listener while the reducer is executing. See https://redux.js.org/api-reference/store#subscribe(listener) for more details.

This error is located at:
    in Connect(n)
    in n
    in t
    in l
    in RCTView
    in Root(native)
    in RCTView
    in RCTView
    in t, stack:
<unknown>@320:3588
tryUnsubscribe@316:1319
componentWillUnmount@313:3148
ur@108:51027
fr@108:53185
si@108:64038
ui@108:61840
oi@108:61224
ri@108:60352
Ir@108:59332
enqueueSetState@108:31972
setState@14:1794
<unknown>@339:10124
<unknown>@475:990
value@475:946
b@677:2955
<unknown>@677:7728
<unknown>@475:990
value@475:946
d@489:710
i@489:1270
<unknown>@320:5006
<unknown>@1130:1359
v@320:4105
<unknown>@512:1380
<unknown>@1142:13183
<unknown>@1141:176
dispatch@320:6106
onSuccess@466:6655
<unknown>@462:826
_@126:151
<unknown>@126:878
<unknown>@29:1801
k@29:643
q@29:1007
callImmediates@29:3202
value@24:2904
<unknown>@24:1377
value@24:2621
value@24:1347
value@24:962

       at com.facebook.react.modules.core.ExceptionsManagerModule.showOrThrowError(ExceptionsManagerModule.java:54)
       at com.facebook.react.modules.core.ExceptionsManagerModule.reportFatalException(ExceptionsManagerModule.java:38)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
       at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:160)
       at com.facebook.react.bridge.queue.NativeRunnable.run(NativeRunnable.java)
       at android.os.Handler.handleCallback(Handler.java:739)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
       at android.os.Looper.loop(Looper.java:158)
       at com.facebook.react.bridge.queue.MessageQueueThreadImpl$3.run(MessageQueueThreadImpl.java:192)
       at java.lang.Thread.run(Thread.java:818)
@mjw56

This comment has been minimized.

Copy link
Contributor Author

mjw56 commented Sep 13, 2018

@phips28 it's hard to say without seeing the app's component code, but I think you might be doing some async action and then immediately unmounting a connected component before the async action has finished?

@markerikson

This comment has been minimized.

Copy link
Contributor

markerikson commented Sep 13, 2018

Looking at the call stack, the unsubscribe is clearly coming from React-Redux's connect function, but this behavior "should" be impossible given how JS works. I see that it's an RN app, so there must be some complicating factor there somehow.

@phips28

This comment has been minimized.

Copy link

phips28 commented Sep 14, 2018

I updated some libraries, maybe there was a breaking change somewhere. I added some more logging to locate the problem, but its difficult because I cannot reproduce it.
Will keep you posted.

@SoundBot SoundBot referenced this pull request Sep 17, 2018

Closed

Migration guide for 4.0 #3138

@phips28

This comment has been minimized.

Copy link

phips28 commented Sep 17, 2018

This is a un-debug-able bug crash 🙈

  1. it only happens on android
  2. only in release mode
  3. but I can reproduce it, just press one button, but this button has no getState()

But the stacktrace doesnt help me lot.
Do you know how I can get more info to solve this issue?
Its really hard to debug when I always have to do a release build and this is minified, any idea how to create a non-minified bundle to see more info?

09-17 22:20:20.459 3714-3816/? E/ReactNativeJS: Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in RCTView
        in t
        in RCTView
        in RCTView
        in n
        in TouchableWithoutFeedback
        in RCTView
        in n
        in t
        in t
        in RCTView
        in n
        in t
        in t
        in t
        in RCTView
        in t
        in r
        in withCachedChildNavigation(r)
        in Unknown
        in n
        in n
        in RCTView
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t
    Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in RCTView
        in t
        in RCTView
        in RCTView
        in n
        in TouchableWithoutFeedback
        in RCTView
        in n
        in t
        in t
        in RCTView
        in n
        in t
        in t
        in t
        in RCTView
        in t
        in r
        in withCachedChildNavigation(r)
        in Unknown
        in n
        in n
        in RCTView
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t
    Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t
09-17 22:20:20.479 3714-3816/? E/ReactNativeJS: Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t
09-17 22:20:20.479 3714-3817/? E/unknown:ReactNative: Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in RCTView
        in t
        in RCTView
        in RCTView
        in n
        in TouchableWithoutFeedback
        in RCTView
        in n
        in t
        in t
        in RCTView
        in n
        in t
        in t
        in t
        in RCTView
        in t
        in r
        in withCachedChildNavigation(r)
        in Unknown
        in n
        in n
        in RCTView
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t, stack:
    y@320:2874
    run@313:4847
    componentWillReceiveProps@313:2959
    In@108:32600
    tr@108:45240
    Nr@108:56528
    Ur@108:56960
    ui@108:61804
    oi@108:61224
    ri@108:60352
    Ir@108:59332
    enqueueSetState@108:31972
    setState@14:1794
    <unknown>@339:6366
    <unknown>@475:990
    value@475:946
    b@677:3023
    <unknown>@677:7864
    <unknown>@475:990
    value@475:946
    d@489:710
    i@489:1270
    <unknown>@320:5006
    <unknown>@1130:1359
    v@320:4105
    <unknown>@512:1380
    <unknown>@1142:13183
    <unknown>@1141:176
    dispatch@320:6106
    onSuccess@466:6615
    <unknown>@462:649
    _@126:151
    <unknown>@126:878
    <unknown>@29:1801
    k@29:643
    q@29:1007
    callImmediates@29:3202
    value@24:2904
    <unknown>@24:1377
    value@24:2621
    value@24:1347
    value@24:962
09-17 22:20:20.489 3714-3817/? E/unknown:ReactNative: Error: You may not call store.getState() while the reducer is executing. The reducer has already received the state as an argument. Pass it down from the top reducer instead of reading it from the store.
    
    This error is located at:
        in Connect(t)
        in RCTView
        in n
        in RCTView
        in t
        in RCTView
        in RCTView
        in n
        in TouchableWithoutFeedback
        in RCTView
        in n
        in t
        in t
        in RCTView
        in n
        in t
        in t
        in t
        in RCTView
        in t
        in r
        in withCachedChildNavigation(r)
        in Unknown
        in n
        in n
        in RCTView
        in RCTView
        in n
        in Connect(n)
        in n
        in t
        in l
        in RCTView
        in Root(native)
        in RCTView
        in RCTView
        in t, stack:
    y@320:2874
    run@313:4847
    componentWillReceiveProps@313:2959
    In@108:32600
    tr@108:45240
    Nr@108:56528
    Ur@108:56960
    ui@108:61804
    oi@108:61224
    ri@108:60352
    Ir@108:59332
    enqueueSetState@108:31972
    setState@14:1794
    <unknown>@339:6366
    <unknown>@475:990
    value@475:946
    b@677:3023
    <unknown>@677:7864
    <unknown>@475:990
    value@475:946
    d@489:710
    i@489:1270
    <unknown>@320:5006
    <unknown>@1130:1359
    v@320:4105
    <unknown>@512:1380
    <unknown>@1142:13183
    <unknown>@1141:176
    dispatch@320:6106
    onSuccess@466:6615
    <unknown>@462:649
    _@126:151
    <unknown>@126:878
    <unknown>@29:1801
    k@29:643
    q@29:1007
    callImmediates@29:3202
    value@24:2904
    <unknown>@24:1377
    value@24:2621
    value@24:1347
    value@24:962
@phips28

This comment has been minimized.

Copy link

phips28 commented Sep 18, 2018

I now disabled minification of the JS bundle and it seems to fix the problem.
I had no idea how to inject the extraArgs in gradle, so I edited this file directly:

PROJECT/node_modules/react-native/react.gradle

commandLine(*nodeExecutableAndArgs, cliPath, bundleCommand, "--platform", "android", "--dev", "${devEnabled}",
                            "--minify", "false", // <<<< fixes the connect() react-redux crash
                            "--reset-cache", "--entry-file", entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir, *extraArgs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment