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

`Cannot read properties of undefined (reading 'defaultPort')` when used with `nock` #675

Closed TiddoLangerak closed 1 week ago

TiddoLangerak commented 4 months ago

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

Describe the bug When using Nock to mock requests to Xero then error responses result in a Cannot read properties of undefined (reading 'defaultPort') error triggered in the ApiError class.

This appears to be caused by xero-node using an undocumented http.ClientRequest.agent property to fetch the port, which doesn't exist when using Nock (and possibly other mocking frameworks, too).

To Reproduce

  1. Setup a test using nock to generate some kind of error response. For example: 
    nock(/xero/)
    .get(/Accounts/)
    .reply(429);
client.accountingApi.getAccounts(/*...*/);
  2. Observe that this triggers the above error, instead of throwing an ApiError

Expected behavior I expect an ApiError to be thrown, instead of a cannot read properties of undefined

Screenshots

Additional context http.ClientRequest.agent is an undocumented property, it's likely not intended as a public facing API. There's a documented alternative available to get the port: request.socket.localPort. So replacing

port: axiosError.request.agent.defaultPort,

with

port: axiosError.request.socket.localPort

should give the same information, using a documented public api, and compatible with Nock.

github-actions[bot] commented 4 months ago

PETOSS-396

github-actions[bot] commented 4 months ago

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

TiddoLangerak commented 3 months ago

Have you had the chance to consider this issue? I'd be happy to submit a PR if you agree with the suggested change?

QuentinLemCode commented 3 months ago

Same issue here

QuentinLemCode commented 3 months ago

@TiddoLangerak The PR can maybe accelerate the fix :)

QuentinLemCode commented 3 months ago

@sangeet-joy-tw Can we have update on this please ?

c4c1us commented 2 months ago

Hey, @sangeet-joy-tw Could we have an update on this, please? Thanks in advance 🙏

QuentinLemCode commented 2 months ago

Hello @BenGDev @GraceYBR @JamesColemanXero @MattR-Api @MJMortimer @TheRegan This issue blocks us to update the xero-node dependency. We rely on nock to test our business services using Xero API.

Can you review and approve this PR please ? https://github.com/XeroAPI/xero-node/pull/682

QuentinLemCode commented 1 month ago

@manishT72 @manishT72x Could you take a look at this issue and the PR linked, please ?

sangeet-joy-tw commented 1 week ago

Hey @TiddoLangerak @QuentinLemCode @c4c1us

We have addressed the port issue in our latest xero-node v7.0.0.

Please install the latest version and kindly let us know if you guys are facing any more "undefined" issues with API Error class.