Should MobX run reactions even if action throws?

[spion]

I have a question:

In the following example:

https://jsfiddle.net/s694ragx/

Despite the action failing, mobx still runs all reactions, showing their errors first - then displays the action error.

In real app situations, this can be very confusing. If there are 10 reactions scheduled to run, the first exception you see will be from the reactions caused by the bad state. Only at the end will you see the actual, original error.

So I have two questions:

• Is it a good idea to run reactions if an action throws and isn't caught?
  • It could be argued that its not, since the app is likely to be in a bad state
• If we need to run reactions, it possible to log the action error before the reaction errors?
  • As a programmer I usually look at the first error which is often the most uesful, but MobX changes the order on me and thats confusing.

[mweststrate]

Not an full answer yet, but it is the result of the combination of this https://github.com/mobxjs/mobx/blob/master/CHANGELOG.md#improve-error-handling and the fact that in MobX reactions are run synchronously. If the reactions wouldn't be run, they would just be sitting there in the queue, and suddenly (and throw!) run if something else, in the future, causes those reactions to be processed. That would be even more unclear, as it would cause exceptions at totally unrelated stack frames (and you might have caught the exception of the original action, not realizing there is anything wrong at all, until in some far future those reactions start throwing...)

[spion]

Ok, I thought something like that may be an issue.

On to the second point then, can we change the order that the exception generated in the action is logged? I.e. we first log the action exception, then run reactions and get exceptions from them, in that way they appear in the order they happened.

This is just to better align with dev expectations - you usually look for the first error in a stream of errors, because the first error that happened is closest to the principal cause.

[spion]

Just forgot to mention that I'm only giving this example because we recently got a stream of 20+ reaction errors and we couldn't make sense looking at the first one why it happened, so instead we logged the actions step by step (binary search 😁) to find the exact spot where it breaks.

Initially I thought the problem is that MobX fully swallowed the error, but then I realized it didn't, it just wasn't logged first but buried near the bottom of the reactions (other unavoidable browser events like mouse movement caused even more errors later, so it was exactly in the middle)

[spion]

AFAICT the way this happens is this implementation:

mobx/src/core/action.ts

Lines 31 to 39 in c74c9f8

	export function executeAction(actionName: string, fn: Function, scope?: any, args?: IArguments) {
	const runInfo = startAction(actionName, fn, scope, args)
	try {
	return fn.apply(scope, args)
	} finally {
	endAction(runInfo)
	}
	}

The finally calls run before the exception fully unwinds, so their exceptions get logged first. Not sure how to solve this one yet.

[mweststrate]

Maybe: eat (or catch, delay, and rethrow) all exceptions that are thrown in the finally block.

Sounds extremely ugly. On the other hand, the exception we are really interested in is indeed the original one. Maybe we could print just a noticed about exceptions that were thrown in the finally part

[mweststrate]

Addressed and released in 4.9.0 / 5.9.0. The above fiddle now looks like:

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.