mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.54k stars 1.77k forks source link

Reaction is not working and observers is emptied incorrectly #2786

Closed Aqours closed 3 years ago

Aqours commented 3 years ago

Intended outcome:

image

Reaction works fine. It will enter runReactions().

Actual outcome:

image

Somtimes, all reactions do not work.

  1. It never enter runReactions() if it has been broken.
  2. Observed variables becomes Unobserved.
  3. Observers have been emptied incorrectly.

How to reproduce the issue:

  1. Invoke function
  2. Using Mobx to change internal state in function
  3. Observing internal state to change view

Run above workflow many times, it sometimes may cause the problem.

Versions 4.15.7

iChenLei commented 3 years ago

Thanks for bug report, We will track this issue.

How to reproduce the issue:

  1. Invoke function
  2. Using Mobx to change internal state in function
  3. Observing internal state to change view

Run above workflow many times, it sometimes may cause the problem.

I think it's not easy to reproduce this issue :(

Could you please provide a minimal reproducation code on codesandbox.io(or other online platform like jsfiddle.net)

Aqours commented 3 years ago

@iChenLei It's difficult to create a minimal repo to reproduce this issue. I'll investigate my project.

mweststrate commented 3 years ago

Do check if you didn't accidentally disabled the error boundaries, which does allow MobX to end up in a corrupted state indeed: https://mobx.js.org/configuration.html#disableerrorboundaries-boolean

Aqours commented 3 years ago

I did not disable error boundaries.

danielkcz commented 3 years ago

@Aqours

It's difficult to create a minimal repo to reproduce this issue. I'll investigate my project.

Imagine, how difficult it is for us to figure out the problem if you can't even reproduce it reliably :) We can't really just "think" of a solution that will magically fix it on your side.

Aqours commented 3 years ago

I can reproduce it with my private project which I cannot share. I will retry to reproduce the issue with minimal repo.

Aqours commented 3 years ago

I think it might be a issue with Chromium. I could reproduce it on Chrome 88 (Win&Mac), Chrome Canary 90 (Win) and Chromium Edge Dev 90 (Win). And it works fine on Firefox 84 (Win) and Safari 14 (Mac).

I will investigate it deeply.

Aqours commented 3 years ago
  1. Debug executeAction() on line 949 at node_modules/mobx/lib/mobx.module.js
    
    // Set runtime id
    let s = 0;

// Cache logs to Window window['d'] = '';

function executeAction(actionName, fn, scope, args) { var runInfo = _startAction(actionName, scope, args);

// Increment runtime id
s++;

try {
    // Log runtime id+try
    window['d'] += `${s},try;`

    return fn.apply(scope, args);
}
catch (err) {
    // Log runtime id+catch
    window['d'] += `${s},catch;`

    runInfo.error = err;
    throw err;
}
finally {
    // Log runtime id+finally+\n
    window['d'] += `${s},finally;\n`

    _endAction(runInfo);
}

}


2. Build my project and reproduce this issue
3. Copy logs from Devtools>Console
4. Use command `grep -oi [TEXT] tmp.txt | wc -l` to check the total of `try`(**32932**), `catch`(0) and `finally`(**32931**)
5. It's strange that there lose one `finally` action

**Debug logs**

https://gist.github.com/Aqours/e8134aba22ee15a02a9b5d179179c052
danielkcz commented 3 years ago

That's not really helpful knowing that one finally out of that many haven't occurred. You will need to dig deeper and find out what is that special case where it happens. I can't think of the reason why could that happen. Perhaps some action that never ends? But that would probably incur other issues.

mweststrate commented 3 years ago

In your test code, if a nested action is called, you will see:

try 1, try 2, finally 2, finally 2. So the irregular output you captured is a bit misleading. For more robust output you will need to keep a local copy of the counter:

s++
const current = s;

And then print current, not s. After that it will probably easier to find if there is a specific s that never saw a finally (e.g. the first or the last).

Aqours commented 3 years ago

@mweststrate Thank you for your suggestion. New logs https://gist.github.com/Aqours/1a15149f2e9907960b04ff0ef6ef9dd2 Line 9655: 9655 no finally

@FredyC What do you mean some action never ends? Would these code cause never ends?

function startFrameLoop() {
    requestAnimationFrame(() => {
        startFrameLoop();
        // Set mobx states
        runInAction(() => {});
    });
}
startFrameLoop();

document.body.onmousemove = function(e) {
    // Set mobx states
    this.lastClientX = e.clientX;
    this.lastClientY = e.clientY;
}

It's ridiculous that I cannot reproduce the issue without using chrome.Devtools.Console. I think it's a bug of chrome.Devtools.Console module.

danielkcz commented 3 years ago

What do you mean some action never ends? Would these code cause never ends?

Just a wild guess. Normally the finally should always run.

However, as I said, your log Is not enough without any means to reproduce it. So if you want it fixed somehow, you will have to work on that unfortunately.

Aqours commented 3 years ago

https://user-images.githubusercontent.com/8586286/107212411-e574fd00-6a41-11eb-9b32-9b36bd4d20e5.mp4

Console will execute the code before return fn.apply(scope, args); when I just paste code.

danielkcz commented 3 years ago

Sorry mate, I know it's hard, but imagine what can we do with it? If we can't reproduce it reliably, ie. write a test that fails. It's impossible to come up with a fix for this. So you can either accept it's a problem of the environment or try to isolate this.

Aqours commented 3 years ago

I confirm it's a bug of chrome, I just put the video here to explain the details.