search

XeroAPI / xero-node

Xero Node SDK for OAuth 2.0 generated from XeroAPI/Xero-OpenAPI
http://developer.xero.com/
MIT License
192 stars 155 forks source link

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

Open grahamsawers opened 3 months ago

grahamsawers commented 3 months ago

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

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

https://github.com/XeroAPI/xero-node/issues/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.

github-actions[bot] commented 3 months ago

PETOSS-401

github-actions[bot] commented 3 months ago

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

grahamsawers commented 3 months ago

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 commented 2 months ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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