xero-node icon indicating copy to clipboard operation
xero-node copied to clipboard

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

Open grahamsawers opened this issue 1 year ago • 7 comments

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:

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.

grahamsawers avatar Mar 09 '24 12:03 grahamsawers

PETOSS-401

github-actions[bot] avatar Mar 09 '24 12:03 github-actions[bot]

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

github-actions[bot] avatar Mar 09 '24 12:03 github-actions[bot]

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.

grahamsawers avatar Mar 11 '24 09:03 grahamsawers

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.

applecran avatar Apr 09 '24 21:04 applecran

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!

jeesus avatar Jun 20 '24 08:06 jeesus

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.

simonhffx avatar Jun 21 '24 09:06 simonhffx

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

applecran avatar Jun 21 '24 16:06 applecran

Any update on this? Will errors once again be errors and not strings?

applecran avatar Feb 22 '25 16:02 applecran

Can @sangeet-joy-tw and @manishT72 have a look at #579 to understand where and why errors are converted to String, I can help but if the authors can bootstrap this effort it's ideal, thanks

caub avatar Mar 03 '25 08:03 caub

Can we revert #662 for now? I'll make a MR for reverting it

caub avatar Mar 18 '25 14:03 caub