ember-changeset icon indicating copy to clipboard operation
ember-changeset copied to clipboard

Type-Error thrown when a JSON-API server responds with a 406 error and an error body

Open psbanka opened this issue 5 years ago • 8 comments
trafficstars

Version

3.9.2

Test Case

Sorry, don't have a test case for this

Steps to reproduce

When saving a changeset, have a server respond with a 406 error response and the following body:

{
  errors: [
    { 
      title: 'Well excuse me',
      detail: 'Error creating the thing you wanted.'
    }
  ]
}

Expected Behavior

I expect the Ember Data exception to be passed to my catch()rather than a type-error from ember-changeset

Actual Behavior

ember-changeset throws a TypeError:

TypeError: Cannot convert undefined or null to object, and it is impossible to get the original error back from Ember Data. Instead, I have to pull all the information out of the changeset.snapshot().changes object, save them to the model manually, and save the model in order to get the Ember Data exception.

psbanka avatar Oct 07 '20 17:10 psbanka

@psbanka 👋 I merged a PR that included improvements to an existing test to more closely match your case. However, I'm still seeing the tests pass. Do you happen to know how your situation might not be reflected in the tests?

https://github.com/poteto/ember-changeset/blob/ae3beb656c7abb645df45552fd113f9ccff7670a/tests/unit/changeset-test.js#L1400

Feel free to reopen the issue if you feel the need to!

snewcomer avatar Oct 10 '20 17:10 snewcomer

I'm running into this as well. Is there a way to work around this?

jherdman avatar Nov 16 '20 21:11 jherdman

Some data I've gathered:

  1. Changeset appears to be valid on the client, and changeset.save() is called
  2. API responds with 422, looks something like this:
{"errors":{"user_email":["[email protected] is already a client"]}}
  1. Deep in the stack of Ember Changeset, you see it failing here:

https://github.com/poteto/ember-changeset/blob/dbf405a9814afdc62b6b4619a36820c68908e30d/addon/utils/merge-deep.js#L61-L81

  1. Inspecting source reveals something like this:
{ userEmail: null, firstName: null, lastName: null, email: null }
  1. The below line is truthy

https://github.com/poteto/ember-changeset/blob/dbf405a9814afdc62b6b4619a36820c68908e30d/addon/utils/merge-deep.js#L75-L77

because typeof null === 'object'

  1. buildPathToValue is called with null as the source

  2. EXPLODE

jherdman avatar Nov 16 '20 22:11 jherdman

I'm running into this too and thank the gods I'm not alone in this, because I've been scratching my head for hours on this one and have not figured out a workaround yet.

@snewcomer Might want to reopen this one

jkeen avatar Nov 17 '20 22:11 jkeen

@jherdman For 6), if possible === null, then the first part of that if expression would fail and we would move on.

Do you happen to know of a good test we could add?

snewcomer avatar Nov 18 '20 04:11 snewcomer

@jherdman For 6), if possible === null, then the first part of that if expression would fail and we would move on.

Merge-deep in ember-changeset and merge-deep in validated-changeset are slightly different, where that expression will fail appropriately in ember-changeset, but not in validated-changeset as typeof(null) == 'object'

https://github.com/poteto/ember-changeset/blob/master/addon/utils/merge-deep.js#L75

Yesterday when stepping through my code (which is using ember-changeset 3.10.1), I was hitting that spot in validated-changeset where it blows up like @jherdman described, when possible is null

https://github.com/validated-changeset/validated-changeset/blob/master/src/utils/merge-deep.ts#L71

Going up a level and starting at the mergeDeep call, it succeeds in the case of merging plain objects, but merging an object with null values { userEmail: null, firstName: null, lastName: null, email: null } and a changeset is where things get hinky.

jkeen avatar Nov 18 '20 14:11 jkeen

Published 3.10.2. Hopefully this resolves the merge-deep issues!

snewcomer avatar Nov 21 '20 05:11 snewcomer

Works for me! Thanks @snewcomer

jkeen avatar Nov 26 '20 13:11 jkeen