Expect a message property at a minimum to be interpreted as an Error
#77
Conversation
| test('should deserialize error', t => { | ||
| const deserialized = deserializeError(new Error('test')); |
There was a problem hiding this comment.
This test was not testing deserialization
| test('should deserialize error', t => { | ||
| const deserialized = deserializeError(new Error('test')); | ||
| const deserialized = deserializeError({ | ||
| message: 'Stuff happened', |
There was a problem hiding this comment.
This is the new minimum viable serialized object
| @@ -169,10 +169,23 @@ test('should deserialize array', t => { | |||
| deserializeNonError(t, [1]); | |||
| }); | |||
|
|
|||
| test('should deserialize empty object', t => { | |||
| deserializeNonError(t, {}); | |||
There was a problem hiding this comment.
serializeError({}) was deserialized as new Error(''), now it throws a NonError
index.js
Outdated
| @@ -78,7 +78,7 @@ const destroyCircular = ({ | |||
| from: value, | |||
| seen: [...seen], | |||
|
|
|||
| to_: isErrorLike(value) ? new Error() : undefined, | |||
| to_: isMinimumViableSerializedError(value) ? new Error() : undefined, | |||
There was a problem hiding this comment.
This fixes the inconsistent expectation between the minimum viable error at the top level and for nested errors.
{}was deserialized as an error{cause: {name: 'Error', message: 'foo'}was serialized as an error, butcausewasn't
After #73 this check should also be changed in the new code there.
fregante
changed the title
message property at a minimum to be interpreted as an Error
|
I think the docs should explicitly mention what we consider an error when deserializing: https://github.com/sindresorhus/serialize-error#deserializeerrorvalue-options |
|
By the way this is another breaking change, technically, because: deserializeError({stuffHappened: "details"})
- // => new Error('') with 'stuffHappened' property
+ // => new NonError('{"stuffHappened": "details"}') |
|
Can you fix the merge conflict? |
No description provided.