apollo-datasource-rest: Dont' parse as json when status code is 204 No Content#2446
Conversation
…son is set as contentType.
|
@bartverbruggen: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
bartverbruggen
requested review from
abernix and
martijnwalraven
as code owners
|
Any idea what could go wrong on the tests? It fails on tests I didn't even made changes on.. I've cleared cache etc... |
|
Seems to be failing for potentially important reasons related to the change?
|
|
@abernix How is that error related to my changes? Even after reverting all my edits, i still get those errors. Loosing it on this one.. I'll close this PR and create an issue, hoping that someone else can fix this small but vital issue. |
|
I'm running the tests locally and I'm seeing the failures as well, however when I revert the two commits in this PR (i.e. |
| contentType && | ||
| (contentType.startsWith('application/json') || | ||
| contentType.startsWith('application/hal+json')) | ||
| response.status !== 204 || |
There was a problem hiding this comment.
Are you sure you didn't mean to put this as an && inside the parenthesis that contain contentType.startsWith(...) || ...?
The failing tests are because of Content-type: text/plain and you're always short-circuiting when the response.status !== 204, which would be most all of the time, no?
There was a problem hiding this comment.
this is embarrassing..., it, indeed, should have been && instead of ||, it is not necessary to put it inside the parenthesis because it should pass all three conditions.
Thank you very much!
There was a problem hiding this comment.
No need to be embarrassed. That's what failing tests and code-reviews are meant to help with. 😄
Thank you for the first-time contribution!
* add no-cache headers to PersistedQuery error responses * fix defaultHeaders typo * remove try/catch on hasPersistedQueryError replace with Array.isArray check * Add CHANGELOG.md for #2446.
|
Thanks for this PR! It's been published in |
|
Fix seems to work. Great job! |
4 participants
No description provided.