Skip to content

Conversation

@theninj4
Copy link
Contributor

If we get a non-conformant response to a request, we should be producing a generic json:api compliant error to pass back up the stack. Right now, we're either passing back objects that don't meet the consumers expectations, or throwing exceptions as we try and access properties that don't exist.

@theninj4 theninj4 added the bug label Jan 26, 2016
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope of this change, but is there any danger JSON.parse(payload.text) will fail to parse and throw and exception? Should we put in a try..catch block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I'll add it.

@pmcnr-hx
Copy link
Contributor

Unit tests seem to be broken on CI...

@theninj4
Copy link
Contributor Author

@pmcnr-hx the failing tests are CORS related on iOS-9.2:

Chrome 47.0.2526 (Windows 10 0.0.0): Executed 23 of 24 (skipped 1) SUCCESS (6.586 secs / 5.747 secs)
Firefox 43.0.0 (Windows 10 0.0.0): Executed 23 of 24 (skipped 1) SUCCESS (5.906 secs / 5.332 secs)
IE 11.0.0 (Windows 10 0.0.0): Executed 23 of 24 (skipped 1) SUCCESS (7.64 secs / 6.769 secs)
IE 9.0.0 (Windows 7 0.0.0): Executed 23 of 24 (skipped 1) SUCCESS (4.727 secs / 3.986 secs)
Mobile Safari 7.0.0 (iOS 7.1.0): Executed 23 of 24 (skipped 1) SUCCESS (7.087 secs / 6.307 secs)
Chrome Mobile 39.0.0 (Android 5.1.0): Executed 23 of 24 (skipped 1) SUCCESS (7.987 secs / 6.056 secs)
Mobile Safari 9.0.0 (iOS 9.2.0): Executed 11 of 24 (10 FAILED) (skipped 1) (2.367 secs / NaN secs)

@theninj4
Copy link
Contributor Author

Tests are still as good as they're going to get 👍

@pmcnr-hx
Copy link
Contributor

All good! 👍 :shipit:

@DuncanFenning
Copy link
Contributor

Nice one 👍

theninj4 pushed a commit that referenced this pull request Jan 26, 2016
Better error handling of non-conformant json:api responses
@theninj4 theninj4 merged commit 60128d1 into master Jan 26, 2016
@theninj4 theninj4 deleted the better-remote-error-handling branch January 26, 2016 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants