Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
psbanka opened this issue Oct 7, 2020 · 8 comments

Comments

@psbanka
Copy link

psbanka commented Oct 7, 2020

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.

@snewcomer
Copy link
Collaborator

snewcomer commented Oct 10, 2020

@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!

@jherdman
Copy link

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

@jherdman
Copy link

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":["digit@example.com 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

@jkeen
Copy link

jkeen commented Nov 17, 2020

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

@snewcomer snewcomer reopened this Nov 18, 2020
@snewcomer
Copy link
Collaborator

@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?

@jkeen
Copy link

jkeen commented Nov 18, 2020

@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.

@snewcomer
Copy link
Collaborator

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

@jkeen
Copy link

jkeen commented Nov 26, 2020

Works for me! Thanks @snewcomer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants