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

[BUGFIX] Issue 7916 - handle invalid response if payload is null #8305

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mani-rsg
Copy link

Description

This fixes issue #7916. Handle an Invalid response though if the payload is null.

Done the change and linked this to a sample Ember app and tested by using a mock API returning status 422 and response as null

@mani-rsg
Copy link
Author

HI @runspired , I tried to add test for this change in tests/main/tests/integration/adapter/handle-response-test.js as below:
But this didn't get failed when I removed the condition that I added in packages/adapter/addon/rest.ts. I was expecting a typeError when payload was passed null. I used this command to test pnpm --filter main-test-app run test.

test('handleResponse is called with correct parameters on null response with 422 status', async function (assert) {
    let handleResponseCalled = 0;

    this.server.get('/people', function () {
      return ['422', { 'Content-Type': 'application/json' }, null];
    });

    class TestAdapter extends JSONAPIAdapter {
      handleResponse(status, headers, payload, requestData) {
        console.log(payload, 'payload');
        handleResponseCalled++;

        return super.handleResponse(status, headers, payload, requestData);
      }
    }

    this.owner.register('adapter:application', TestAdapter);

    try {
      await this.store.findAll('person');
      assert.ok(false, 'promise should reject');
    } catch {
      assert.ok(true, 'promise rejected');
    }

    assert.strictEqual(handleResponseCalled, 1, 'handle response is called');
  });

@runspired runspired added 🎯 canary PR is targeting canary (default) 🏷️ bug This PR primarily fixes a reported issue labels Nov 14, 2022
@runspired
Copy link
Contributor

@mani-rsg let's still add that test but we will need to tweak it to ensure the error we are catching is the appropriate error. Since in both cases we throw, we just throw something unexpected in the payload=null case.

@runspired runspired linked an issue Dec 7, 2022 that may be closed by this pull request
@runspired
Copy link
Contributor

@mani-rsg were you still interested in this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) good-first-issue 🏷️ bug This PR primarily fixes a reported issue
Projects
Status: stalled
Development

Successfully merging this pull request may close these issues.

Error if payload===null
3 participants