Skip to content

Commit

Permalink
Improved error logging, suppress reaction errors if action is already…
Browse files Browse the repository at this point in the history
… throwing. Fixes #1836
  • Loading branch information
mweststrate committed Jan 21, 2019
1 parent 19ec170 commit 41ba822
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 12 deletions.
13 changes: 11 additions & 2 deletions src/core/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,19 @@ export function createAction(actionName: string, fn: Function): Function & IActi

export function executeAction(actionName: string, fn: Function, scope?: any, args?: IArguments) {
const runInfo = startAction(actionName, fn, scope, args)
let shouldSupressReactionError = true
try {
return fn.apply(scope, args)
const res = fn.apply(scope, args)
shouldSupressReactionError = false
return res
} finally {
endAction(runInfo)
if (shouldSupressReactionError) {
globalState.suppressReactionErrors = shouldSupressReactionError
endAction(runInfo)
globalState.suppressReactionErrors = false
} else {
endAction(runInfo)
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/core/globalstate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,12 @@ export class MobXGlobals {
* the stack when an exception occurs while debugging.
*/
disableErrorBoundaries = false

/*
* If true, we are already handling an exception in an action. Any errors in reactions should be supressed, as
* they are not the cause, see: https://github.com/mobxjs/mobx/issues/1836
*/
suppressReactionErrors = false
}

let canMergeGlobalState = true
Expand Down
10 changes: 7 additions & 3 deletions src/core/reaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,13 @@ export class Reaction implements IDerivation, IReactionPublic {

if (globalState.disableErrorBoundaries) throw error

const message = `[mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: '${this}`
console.error(message, error)
/** If debugging brought you here, please, read the above message :-). Tnx! */
const message = `[mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: '${this}'`
if (globalState.suppressReactionErrors) {
console.warn(`[mobx] (error in reaction '${this.name}' suppressed, fix error of causing action below)`) // prettier-ignore
} else {
console.error(message, error)
/** If debugging brought you here, please, read the above message :-). Tnx! */
}

if (isSpyEnabled()) {
spyReport({
Expand Down
15 changes: 11 additions & 4 deletions test/base/__snapshots__/action.js.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`error logging, #1836 1`] = `
", [mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[Autorun@71], Error: Reaction error
, Error: Action error
"
exports[`error logging, #1836 - 1 1`] = `
Array [
"[warn] [mobx] (error in reaction 'Autorun@71' suppressed, fix error of causing action below)",
"[error] Error: Action error",
]
`;

exports[`error logging, #1836 - 2 1`] = `
Array [
"[error] [mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[Autorun@74]'",
]
`;
2 changes: 1 addition & 1 deletion test/base/__snapshots__/spy.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Array [
},
Object {
"error": "Oops",
"message": "[mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[autorun]",
"message": "[mobx] Encountered an uncaught exception that was thrown by a reaction or observer component, in: 'Reaction[autorun]'",
"name": "autorun",
"type": "error",
},
Expand Down
25 changes: 23 additions & 2 deletions test/base/action.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ test("Fix #1367", () => {
expect(mobx.isAction(x.method)).toBe(true)
})

test("error logging, #1836", () => {
const messages = utils.consoleError(() => {
test("error logging, #1836 - 1", () => {
const messages = utils.supressConsole(() => {
try {
const a = mobx.observable.box(3)
mobx.autorun(() => {
Expand All @@ -520,6 +520,27 @@ test("error logging, #1836", () => {
throw new Error("Action error")
})()
} catch (e) {
expect(e.toString()).toEqual("Error: Action error")
console.error(e)
}
})

expect(messages).toMatchSnapshot()
})

test("error logging, #1836 - 2", () => {
const messages = utils.supressConsole(() => {
try {
const a = mobx.observable.box(3)
mobx.autorun(() => {
if (a.get() === 4) throw new Error("Reaction error")
})

mobx.action(() => {
a.set(4)
})()
} catch (e) {
expect(e.toString()).toEqual("Error: Action error")
console.error(e)
}
})
Expand Down

0 comments on commit 41ba822

Please sign in to comment.