Better error messages

[davecoates]

trafficstars

After using typed-immutable on multiple projects now the biggest problem that comes up is tracking down why a type error occurs - particularly when restoring a large nested structure from a raw JS object.

As an example:

const Address = Record({
  line1: String,
  city: String,
  country: String,
});

const Person = Record({
  id: Number,
  name: String,
  email: String,
  age: Number,
  address: Address,
});

const People = new Map(Number, Person);

new People([[1, { 
  id: 1,
  name: 'Bob',
  email: '[email protected]',
  age: 100,
  address: {
    line1: 5,
  }
}]]);

currently gives an error

TypeError: Invalid value: Invalid value for "address" field:
 Invalid value for "line1" field:
 "undefined" is not a string

This is ok when you have the context and know what structures it's referring to be in many cases you don't or the fields are generic enough that it's hard to identify where it comes from. It's also very useful to know the data that failed.

I've been looking at improving them, eg. the above is now:

    TypeError: Entry in Map(Number, Person) failed to satisfy value type:

    Key:
    1

    Value:
    {
      "id": 1,
      "name": "Bob",
      "email": "[email protected]",
      "age": 100,
      "address": {
        "line1": 5
      }
    }

    Failed to create instance of Person

      Value for field 'address' must satisfy type 'Address'

    Failed to create instance of Address

      Value for field 'line1' must satisfy type 'String'

    Invalid value: "5" is not a string

My only concern with adding this is that it will be a bit slower to generate this level of detail (eg. doing a JSON.stringify on value). In my use cases so far it wouldn't matter as I never catch the TypeError's - if they occur it's a bug that we fix.

Does anyone have any thoughts about this? Should it be opt in? Am I overthinking it? I also haven't measured anything yet - I think it may only become a problem if you were generating a lot of these (eg. iterating a large list attempting to instantiating record's and handling any errors).

@lukesneeringer @stutrek @udnisap

[stutrek]

I am always in favor of more descriptive error messages. If the speed of generating errors is holding back your app, something went wrong long before you chose to use typed-immutable. I do think the JSON should be opt in, since you might be logging a gigantic array of gigantic objects and the rest of your text should help find the error just fine.

I don't know if it's the line breaks or if it's longer than I was expecting, but can the message be made more concise? Even if not, I'm in favor.

Do you have a PR for it?

[davecoates]

No PR yet, just basic implementation for Maps / Records, need to extend it to other types.

It has occurred to me that Union's rely on generating TypeErrors so will need to consider that in final implementation.

In terms of size of message it's currently giving you the full trace of each type that failed to instantiate and what the expected type was. Perhaps would be better to just give the path:

TypeError: Entry in Map(Number, Person) failed to satisfy value type for path 'address -> line1':

Invalid value: "5" is not a string

    Key:
    1

    Value:
    {
      "id": 1,
      "name": "Bob",
      "email": "[email protected]",
      "age": 100,
      "address": {
        "line1": 5
      }
    }

The stuff below Invalid value: "5" is not a string could be made optional somehow as that's potentially problematic for large data structures. Not sure best way to do that though - global opt in of some sort?

import { setVerboseDebugging } from 'typed-immutable';

setVerboseDebugging(true);

[lukesneeringer]

I would really like this change also.