Improved error messages

[evelant]

Would it be easy to add more data to runtime error messages? I'm finding with a large state tree that the current error messages give very little information to help locate the problem.

For example when data doesn't match in fromSnapshot you get an error like [Error: TypeCheckError: [/statusEffectDocId] Expected: string] which doesn't contain enough information to effectively figure out what bit of data is wrong.

I think errors on fields would be much more useful if they also contained the following:

1. The invalid value. Currently it's impossible to tell why a type check failed because the invalid value isn't included.
2. The full path to the invalid value from the root of whatever is being instantiated. Most of the time these errors come up when instantiating large nested models from snapshots. For example the above /statusEffectDocId would be much more useful if it were something like (fromSnapshot: PlayerModel) /items/equippedLeftHand/activeStatusEffects/statusEffectDocId. Without the full path it's not possible to know exactly where the invalid value is.
3. The full path up to the root store if assigning a value to a prop in an existing store fails

Poking around the code I'm having trouble figuring out if any of this information is easily available to include in errors. Thoughts @xaviergonz ? Would this potentially be easy? The current error messages give so little context info that it takes a lot of time and effort to track down what's going wrong even though mobx-keystone likely has all that information internally.

[evelant]

Another example: https://github.com/xaviergonz/mobx-keystone/blob/09c8bd479c4a6f465d89e8821772f68979e435ef/packages/lib/src/snapshot/reconcileSnapshot.ts#L55

just unsupported snapshot: [object Object] gives no useful information to help debug why the snapshot is bad or where the snapshot is being instantiated

[xaviergonz]

The invalid value. Currently it's impossible to tell why a type check failed because the invalid value isn't included.

That just got added in 1.2.0

2. The full path to the invalid value from the root of whatever is being instantiated. Most of the time these errors come up when instantiating large nested models from snapshots. For example the above /statusEffectDocId would be much more useful if it were something like (fromSnapshot: PlayerModel) /items/equippedLeftHand/activeStatusEffects/statusEffectDocId. Without the full path it's not possible to know exactly where the invalid value is.
3. The full path up to the root store if assigning a value to a prop in an existing store fails

I think this should be already the case. Do you have a codesandbox that shows this problem?

throw failure(unsupported snapshot - ${sn})

In that particular case, if you get there it usually means whatever you passed to fromSnapshot was not even JSON-compatible (a map, a set, an instance of a date...)

[evelant]

@xaviergonz here's an example https://codesandbox.io/s/xenodochial-cookies-21lwg3?file=/src/model.ts

import { fromSnapshot, Model, model, tProp, types as t } from "mobx-keystone";

@model("TestModel3")
class TestModel3 extends Model({
  test3_1: tProp(t.string)
}) {}

@model("TestModel2")
class TestModel2 extends Model({
  test2_1: tProp(t.string),
  test2_2: tProp(t.model(TestModel3))
}) {}

@model("TestModel1")
class TestModel1 extends Model({
  test1_1: tProp(t.string),
  test1_2: tProp(t.model(TestModel2))
}) {}

const snapshot: { [key: string]: any } = {
  $modelType: "TestModel1",
  test1_1: "asdf",
  test1_2: {
    $modelType: "TestModel2",
    test2_1: "fdas",
    test2_2: { $modelType: "TestModel3", test3_1: 123 }
  }
};
export const test = fromSnapshot(TestModel1, snapshot);

That will throw the error Error: TypeCheckError: [/test3_1] Expected: string. It only contains the path within the enclosing model, not the path all the way back to the type being instantiated. This is what can make it very difficult to know where something is wrong.

If I have a large snapshot with many different layers and types of nested models it can be impossible or very difficult to know where the problem came from. The above example is basic but some places in my app load snapshots with hundreds of keys, some nested inside maps or arrays. Without the full path to the root type being instantiated it's difficult to locate the bad data given just the prop name.

It would also be super helpful if mobx-keystone printed the modelIds (if it has it) for a failed typecheck on any property of a model. That would make it much easier to find the specific piece of bad data. Knowing that one field is bad doesn't help much if it's one field on one item in an array of 400 snaphots. A modelId would the error message would greatly help with finding exactly where the data is wrong.

In the above example, I'd love to see an error message like this if possible: Error: TypecheckError instantiating "TestModel1" at root (type: TestModel1 , modelId: 123) -> test2_2 (type: TestModel2, modelId: 234) -> test_3_1 (type: TestModel3, modelId: 456). Property "test_3_1" on model "TestModel3" with id 456 expected "string", got 123 (number)

An error like that example gives all the information necessary to know exactly why the type didn't match and exactly where the piece of data is based on the snapshot you're constructing from.

Does that help clarify the issue?

[evelant]

@xaviergonz here's an actual example from my app

Error [[snapshot '{"length":3}' does not match the following type: Array<string> | undefined | null]] Error adding activitiesLists id "sXuV8ZqMxTXvzb6EyXFH3AqaYgr1_01GFS9TYTFXQ4V9B8P3CC3GXRG"

The model in question, ActivitiesList, has over 100 properties and many nested sub-models. This error doesn't give me much of a clue about where exactly the bad data is located. All I know from this error is that some optional string array property is getting invalid input.

It would be much more helpful if mobx-keystone would identify the name of the invalid field along with the full path to the root of whatever is being constructed (in the case of sub-models inside of sub-models etc)

[evelant]

I'm still struggling with this fairly frequently. Here's another example Error: snapshot '"2"' does not match the following type: number | undefined | null. I have no idea what property on what model has the invalid value of "2". It could be any of hundreds of properties inside nested models of the one I'm constructing from a snapshot.