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

feat: parsing body for failed ajax responses #2039

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

jorenbroekema
Copy link
Collaborator

What I did

  1. Add automatic response body parsing for fetchJson failed responses
  2. Add test for manually parsing body for fetch failed responses

@changeset-bot
Copy link

changeset-bot bot commented Jul 17, 2023

🦋 Changeset detected

Latest commit: 7e410f2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lion/ajax Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jorenbroekema jorenbroekema merged commit 7a875ef into master Jul 18, 2023
4 checks passed
@jorenbroekema jorenbroekema deleted the ajax-error-reading branch July 18, 2023 11:44
@thepassle
Copy link
Member

This contains a bug when using ajaxApi.fetchJson()

fechJson internally calls this.fetch, where parseErrorResponse is set to true:

const response = await this.fetch(info, jsonInit, true);

However, it doesnt first call fetch on the Ajax class, but on the AjaxApi class, which signature looks like this:

fetch(apiPath, init) {} 

The AjaxApi class's fetch method then calls:

super.fetch(getApiUrl(apiPath), {
  credentials: "include",
  ...init,
});

reaching Ajax's fetch method, where parseErrorResponse default to false:

async fetch(info, init, parseErrorResponse = false) {}

Meaning that it doesn't parse the body for failed requests when using AjaxApi. Can you take a look at this? :)

@jorenbroekema
Copy link
Collaborator Author

@thepassle this is related to ING's extension of @lion/ajax right? I got a task on my list to perpetuate this change to the ING extension and make sure the param is passed

@thepassle
Copy link
Member

Ah yes indeed, this is in AjaxApi which is in ING. My bad. Still need to fix it there too though :)

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

Successfully merging this pull request may close these issues.

3 participants