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

Switch from request to axios is a breaking change? #678

Open
grahamsawers opened this issue Mar 9, 2024 · 7 comments
Open

Switch from request to axios is a breaking change? #678

grahamsawers opened this issue Mar 9, 2024 · 7 comments

Comments

@grahamsawers
Copy link

SDK you're using (please complete the following information):

  • Version >=5.0.0

Describe the bug
Release notes for version 5.0.0 list a bug fix:

#579 | Get rid of deprecated "request" dependency

Switching out the request package for axios changes the response shape for the SDK API methods. Previously they returned a request response whereas now they return an AxiosResponse.

As you can see these responses have a different shape. IMHO changing the response shape of the SDK should be explicitly called out as a breaking change. We didn't pick this up until we noticed runtime errors in production.

Copy link

github-actions bot commented Mar 9, 2024

PETOSS-401

Copy link

github-actions bot commented Mar 9, 2024

Thanks for raising an issue, a ticket has been created to track your request

@grahamsawers
Copy link
Author

grahamsawers commented Mar 11, 2024

I've also noticed that when the request fails the response returned by the SDK is mapped to a Response class and an error is constructed from this.

This response object has a similar shape to the original request response returned on success and not an AxiosResponse. I feel the response object for both error and success should have the same shape.

@applecran
Copy link

The errors are also being thrown as a string now - JSON.Stringify is being called on the error object where it used to be a JSON object.

@jeesus
Copy link

jeesus commented Jun 20, 2024

The errors are also being thrown as a string now - JSON.Stringify is being called on the error object where it used to be a JSON object.

I confirm. This should have been mentioned in the release notes!

@simonhffx
Copy link

simonhffx commented Jun 21, 2024

I confirm. This should have been mentioned in the release notes!

Same here. We are now having issues categorising errors from our Xero integrations because thrown errors are now strings for some reason.

@applecran
Copy link

Yes we rolled back from this release, waiting for an update.

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