Skip to content

Commit 420f48e

Browse files
authored
Improve type/snapshot error diagnostics with full path context (#570)
* Improve type/snapshot error diagnostics with full path context * Add focused tests for snapshot/type diagnostics coverage * Trim diagnostic coverage tests while keeping branch coverage * update changelog
1 parent 03c7912 commit 420f48e

38 files changed

+1537
-302
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
## Unreleased
44

5+
- `TypeCheckError` now supports a single object parameter and still accepts positional constructor arguments for backward compatibility.
6+
- `TypeCheckError.throw()` now throws `TypeCheckErrorFailure` (which extends `MobxKeystoneError`), and union snapshot mismatches now throw `SnapshotTypeMismatchError` (also extending `MobxKeystoneError`) with structured metadata.
7+
- Snapshot processing now throws a dedicated `SnapshotProcessingError` (extends `MobxKeystoneError`) across `fromSnapshot` / `applySnapshot` / reconciliation paths.
8+
- `SnapshotProcessingError` messages now include enriched diagnostics text (full deep path, value preview, and model trail when available), making it much easier to pinpoint failing nested fields quickly.
9+
- `SnapshotProcessingError`, `SnapshotTypeMismatchError`, and `TypeCheckError` now share one diagnostics message formatter and a consistent layout: `MSG - Path: ... - Value: ... - Model trail: ...`, so error output is predictable across snapshot and type-check flows.
10+
- Improved snapshot/type error diagnostics end-to-end: path-aware messages across `fromSnapshot` / `applySnapshot` / union snapshot processing, plus structured error metadata (`path`, `expectedTypeName`, `actualValue`, `typeCheckedValue`, `modelTrail`) on thrown errors when available for easier tooling and debugging.
11+
- Note: because diagnostics were improved, error message text changed in these paths (`TypeCheckError`, `SnapshotProcessingError`, `SnapshotTypeMismatchError`, and related snapshot/apply patches flows). If you assert exact error strings, update expectations.
12+
- `applyPatches` reconciliation errors now also include accurate patch paths in diagnostics.
13+
514
## 1.13.0
615

716
- Added array syntax as a `tProp` union shorthand, for example `tProp([String, Number])` as an alias for `tProp(types.or(String, Number))`.

apps/site/docs/runtimeTypeChecking.mdx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,11 @@ In all cases the returned value will be `null` if there are no errors or an inst
6262
- `path: Path` - Sub-path where the type-check failed, or an empty array if the actual object/value failed the type-check.
6363
- `expectedTypeName: string` - String representation of the expected type.
6464
- `actualValue: any` - The actual value/sub-value that failed the type-check
65-
- `throw(typeCheckedValue: any)` - Throws the error as an exception.
65+
- `typeCheckedValue?: any` - The value/object where the type-check was invoked.
66+
- `modelTrail?: readonly string[]` - Optional model/type trail (for example during snapshot processing of nested models).
67+
- `throw()` - Throws a `TypeCheckErrorFailure` as an exception.
68+
69+
`TypeCheckErrorFailure` extends `MobxKeystoneError`, so `instanceof MobxKeystoneError` checks work on thrown failures.
6670

6771
While models are usually automatically type-checked, it is worth noting that other values (primitives, plain objects, arrays) are not until they become attached to some model. If you need to type-check those before they become attached to a model it is always possible to use `typeCheck(type, value)` as shown previously to trigger a manual validation.
6872

@@ -139,6 +143,7 @@ const shapeType = types.or(
139143
```
140144

141145
The dispatcher receives the snapshot and must return one of the provided union types.
146+
When no union branch matches during snapshot conversion, `types.or` throws `SnapshotTypeMismatchError` (a `MobxKeystoneError`) with path/type/value metadata.
142147

143148
### `types.maybe(type)`
144149

@@ -290,7 +295,13 @@ const sumModelType = types.refinement(types.model(Sum), (sum) => {
290295
return rightResult
291296

292297
// this will return that the result field is wrong
293-
return rightResult ? null : new TypeCheckError(["result"], "a+b", sum.result)
298+
return rightResult
299+
? null
300+
: new TypeCheckError({
301+
path: ["result"],
302+
expectedTypeName: "a+b",
303+
actualValue: sum.result,
304+
})
294305
})
295306
```
296307

apps/site/docs/snapshots.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ applySnapshot(todo, {
194194
195195
In the case above, only a single patch will be generated (for the `done` property), and the same todo instance will be reused (since they have the same model ID).
196196
197+
When `fromSnapshot` or `applySnapshot` fails due to snapshot processing issues, it throws `SnapshotProcessingError` (extends `MobxKeystoneError`) and includes diagnostic metadata such as `path`, `actualSnapshot` (when available), and `modelTrail` (when available). Its error message is also enriched with this metadata (plus a safe value preview when available).
198+
197199
## Cloning via snapshots
198200
199201
### `clone`

packages/lib/src/modelShared/sharedInternalModel.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type { AnyType } from "../types/schemas"
1515
import { tProp } from "../types/tProp"
1616
import type { LateTypeChecker } from "../types/TypeChecker"
1717
import { typesUnchecked } from "../types/utility/typesUnchecked"
18+
import { withErrorPathSegment } from "../utils/errorDiagnostics"
1819
import { addHiddenProp, assertIsObject, failure, propNameToSetterName } from "../utils"
1920
import { chainFns } from "../utils/chainFns"
2021
import { ModelClass, modelInitializedSymbol } from "./BaseModelShared"
@@ -75,7 +76,9 @@ const setModelInstanceDataField: SetModelInstanceDataFieldFn = (
7576
}
7677

7778
let untransformedValue = modelProp._transform
78-
? modelProp._transform.untransform(value, model, modelPropName)
79+
? withErrorPathSegment(modelPropName, () =>
80+
modelProp._transform!.untransform(value, model, modelPropName)
81+
)
7982
: value
8083

8184
// apply default value if applicable
@@ -354,7 +357,9 @@ function getModelPropsFromSnapshotProcessor(
354357
const newSn = { ...sn }
355358
for (const [propName, propData] of propsWithFromSnapshotProcessor) {
356359
if (propData._fromSnapshotProcessor) {
357-
newSn[propName] = propData._fromSnapshotProcessor(sn[propName])
360+
newSn[propName] = withErrorPathSegment(propName, () =>
361+
propData._fromSnapshotProcessor!(sn[propName])
362+
)
358363
}
359364
}
360365
return newSn
@@ -376,7 +381,7 @@ function getModelPropsToSnapshotProcessor(
376381
const newSn = { ...sn }
377382
for (const [propName, propData] of propsWithToSnapshotProcessor) {
378383
if (propData._toSnapshotProcessor) {
379-
newSn[propName] = propData._toSnapshotProcessor(sn[propName])
384+
newSn[propName] = withErrorPathSegment(propName, () => propData._toSnapshotProcessor!(sn[propName]))
380385
}
381386
}
382387
return newSn

packages/lib/src/patch/applyPatches.ts

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { Patch } from "../patch/Patch"
88
import { reconcileSnapshot } from "../snapshot/reconcileSnapshot"
99
import { assertTweakedObject } from "../tweaker/core"
1010
import { failure, inDevMode, isArray, lazy } from "../utils"
11+
import { runWithErrorDiagnosticsContext, withErrorPathSegments } from "../utils/errorDiagnostics"
1112
import { ModelPool } from "../utils/ModelPool"
1213
import { setIfDifferent } from "../utils/setIfDifferent"
1314

@@ -40,36 +41,38 @@ export function internalApplyPatches(
4041
patches: ReadonlyArray<Patch> | ReadonlyArray<ReadonlyArray<Patch>>,
4142
reverse = false
4243
): void {
43-
const obj = this
44-
const modelPool = new ModelPool(obj)
45-
46-
if (reverse) {
47-
let i = patches.length
48-
while (i--) {
49-
const p = patches[i]
50-
if (isArray(p)) {
51-
let j = p.length
52-
while (j--) {
53-
applySinglePatch(obj, p[j], modelPool)
44+
runWithErrorDiagnosticsContext(() => {
45+
const obj = this
46+
const modelPool = new ModelPool(obj)
47+
48+
if (reverse) {
49+
let i = patches.length
50+
while (i--) {
51+
const p = patches[i]
52+
if (isArray(p)) {
53+
let j = p.length
54+
while (j--) {
55+
applySinglePatchWithPath(obj, p[j], modelPool)
56+
}
57+
} else {
58+
applySinglePatchWithPath(obj, p as Patch, modelPool)
5459
}
55-
} else {
56-
applySinglePatch(obj, p as Patch, modelPool)
5760
}
58-
}
59-
} else {
60-
const len = patches.length
61-
for (let i = 0; i < len; i++) {
62-
const p = patches[i]
63-
if (isArray(p)) {
64-
const len2 = p.length
65-
for (let j = 0; j < len2; j++) {
66-
applySinglePatch(obj, p[j], modelPool)
61+
} else {
62+
const len = patches.length
63+
for (let i = 0; i < len; i++) {
64+
const p = patches[i]
65+
if (isArray(p)) {
66+
const len2 = p.length
67+
for (let j = 0; j < len2; j++) {
68+
applySinglePatchWithPath(obj, p[j], modelPool)
69+
}
70+
} else {
71+
applySinglePatchWithPath(obj, p as Patch, modelPool)
6772
}
68-
} else {
69-
applySinglePatch(obj, p as Patch, modelPool)
7073
}
7174
}
72-
}
75+
})
7376
}
7477

7578
const wrappedInternalApplyPatches = lazy(() =>
@@ -149,6 +152,12 @@ function applySinglePatch(obj: object, patch: Patch, modelPool: ModelPool): void
149152
}
150153
}
151154

155+
function applySinglePatchWithPath(obj: object, patch: Patch, modelPool: ModelPool): void {
156+
withErrorPathSegments(patch.path, () => {
157+
applySinglePatch(obj, patch, modelPool)
158+
})
159+
}
160+
152161
function pathArrayToObjectAndProp(
153162
obj: object,
154163
path: Patch["path"]
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { Path } from "../parent/pathTypes"
2+
import { MobxKeystoneError } from "../utils"
3+
import {
4+
buildErrorMessageWithDiagnostics,
5+
getErrorModelTrailSnapshot,
6+
getErrorPathSnapshot,
7+
noErrorValuePreview,
8+
} from "../utils/errorDiagnostics"
9+
10+
export interface SnapshotProcessingErrorData {
11+
message: string
12+
path?: Path
13+
actualSnapshot?: any
14+
modelTrail?: readonly string[]
15+
}
16+
17+
/**
18+
* Thrown when a structural issue is encountered while processing a snapshot (extends `MobxKeystoneError`).
19+
*
20+
* Use `instanceof SnapshotProcessingError` to distinguish snapshot processing errors
21+
* from other `MobxKeystoneError` instances.
22+
*/
23+
export class SnapshotProcessingError extends MobxKeystoneError {
24+
readonly path: Path
25+
readonly actualSnapshot?: any
26+
readonly modelTrail?: readonly string[]
27+
28+
constructor(data: SnapshotProcessingErrorData) {
29+
const path = data.path ?? getErrorPathSnapshot() ?? []
30+
const modelTrail = data.modelTrail ?? getErrorModelTrailSnapshot()
31+
32+
super(
33+
buildErrorMessageWithDiagnostics({
34+
message: data.message,
35+
path,
36+
previewValue: "actualSnapshot" in data ? data.actualSnapshot : noErrorValuePreview,
37+
modelTrail,
38+
})
39+
)
40+
41+
this.path = path
42+
this.actualSnapshot = data.actualSnapshot
43+
this.modelTrail = modelTrail
44+
}
45+
}

packages/lib/src/snapshot/applySnapshot.ts

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,14 @@ import { getSnapshotModelType, isModel } from "../model/utils"
1010
import type { ModelClass } from "../modelShared/BaseModelShared"
1111
import { getModelInfoForName, modelInfoByClass } from "../modelShared/modelInfo"
1212
import { assertTweakedObject } from "../tweaker/core"
13+
import { assertIsObject, inDevMode, isArray, isMap, isPlainObject, isSet, lazy } from "../utils"
1314
import {
14-
assertIsObject,
15-
failure,
16-
inDevMode,
17-
isArray,
18-
isMap,
19-
isPlainObject,
20-
isSet,
21-
lazy,
22-
} from "../utils"
15+
runWithErrorDiagnosticsContext,
16+
} from "../utils/errorDiagnostics"
2317
import { ModelPool } from "../utils/ModelPool"
2418
import { reconcileSnapshot } from "./reconcileSnapshot"
2519
import type { SnapshotInOf, SnapshotOutOf } from "./SnapshotOf"
20+
import { SnapshotProcessingError } from "./SnapshotProcessingError"
2621

2722
/**
2823
* Applies a full snapshot over an object, reconciling it with the current contents of the object.
@@ -40,7 +35,9 @@ export function applySnapshot(node: object, snapshot: unknown): void {
4035
assertTweakedObject(node, "node")
4136
assertIsObject(snapshot, "snapshot")
4237

43-
wrappedInternalApplySnapshot().call(node, snapshot)
38+
runWithErrorDiagnosticsContext(() => {
39+
wrappedInternalApplySnapshot().call(node, snapshot)
40+
})
4441
}
4542

4643
/**
@@ -58,22 +55,31 @@ export function internalApplySnapshot<T extends object>(
5855

5956
if (inDevMode) {
6057
if (ret !== obj) {
61-
throw failure("assertion failed: reconciled object has to be the same")
58+
throw new SnapshotProcessingError({
59+
message: "assertion failed: reconciled object has to be the same",
60+
actualSnapshot: sn,
61+
})
6262
}
6363
}
6464
}
6565

6666
if (isArray(sn)) {
6767
if (!isArray(obj)) {
68-
throw failure("if the snapshot is an array the target must be an array too")
68+
throw new SnapshotProcessingError({
69+
message: "if the snapshot is an array the target must be an array too",
70+
actualSnapshot: sn,
71+
})
6972
}
7073

7174
reconcile()
7275
return
7376
}
7477

7578
if (isFrozenSnapshot(sn)) {
76-
throw failure("applySnapshot cannot be used over frozen objects")
79+
throw new SnapshotProcessingError({
80+
message: "applySnapshot cannot be used over frozen objects",
81+
actualSnapshot: sn,
82+
})
7783
}
7884

7985
// adapt snapshot to target model if possible
@@ -86,32 +92,42 @@ export function internalApplySnapshot<T extends object>(
8692
if (modelType !== undefined) {
8793
const modelInfo = getModelInfoForName(modelType)
8894
if (!modelInfo) {
89-
throw failure(`model with name "${modelType}" not found in the registry`)
95+
throw new SnapshotProcessingError({
96+
message: `model with name "${modelType}" not found in the registry`,
97+
actualSnapshot: sn,
98+
})
9099
}
91100

92101
// we don't check by actual instance since the class might be a different one due to hot reloading
93102
if (!isModel(obj)) {
94103
// not a model instance, no reconciliation possible
95-
throw failure(`the target for a model snapshot must be a model instance`)
104+
throw new SnapshotProcessingError({
105+
message: "the target for a model snapshot must be a model instance",
106+
actualSnapshot: sn,
107+
})
96108
}
97109

98110
if (obj[modelTypeKey] !== modelType) {
99111
// different kind of model, no reconciliation possible
100-
throw failure(
101-
`snapshot model type '${modelType}' does not match target model type '${
112+
throw new SnapshotProcessingError({
113+
message: `snapshot model type '${modelType}' does not match target model type '${
102114
(obj as any)[modelTypeKey]
103-
}'`
104-
)
115+
}'`,
116+
actualSnapshot: sn,
117+
})
105118
}
106119

107120
const modelIdPropertyName = getModelIdPropertyName(modelInfo.class as ModelClass<AnyModel>)
108121
if (modelIdPropertyName) {
109122
const id = (sn as any)[modelIdPropertyName]
110123
if (obj[modelIdKey] !== id) {
111124
// different id, no reconciliation possible
112-
throw failure(
113-
`snapshot model id '${id}' does not match target model id '${obj[modelIdKey]}'`
114-
)
125+
throw new SnapshotProcessingError({
126+
message: `snapshot model id '${id}' does not match target model id '${
127+
obj[modelIdKey]
128+
}'`,
129+
actualSnapshot: sn,
130+
})
115131
}
116132
}
117133

@@ -122,22 +138,34 @@ export function internalApplySnapshot<T extends object>(
122138
if (isPlainObject(sn)) {
123139
if (!(isPlainObject(obj) || isObservableObject(obj))) {
124140
// no reconciliation possible
125-
throw failure("if the snapshot is an object the target must be an object too")
141+
throw new SnapshotProcessingError({
142+
message: "if the snapshot is an object the target must be an object too",
143+
actualSnapshot: sn,
144+
})
126145
}
127146

128147
reconcile()
129148
return
130149
}
131150

132151
if (isMap(sn)) {
133-
throw failure("a snapshot must not contain maps")
152+
throw new SnapshotProcessingError({
153+
message: "a snapshot must not contain maps",
154+
actualSnapshot: sn,
155+
})
134156
}
135157

136158
if (isSet(sn)) {
137-
throw failure("a snapshot must not contain sets")
159+
throw new SnapshotProcessingError({
160+
message: "a snapshot must not contain sets",
161+
actualSnapshot: sn,
162+
})
138163
}
139164

140-
throw failure(`unsupported snapshot - ${sn}`)
165+
throw new SnapshotProcessingError({
166+
message: "unsupported snapshot",
167+
actualSnapshot: sn,
168+
})
141169
}
142170

143171
const wrappedInternalApplySnapshot = lazy(() =>

0 commit comments

Comments
 (0)