idempotencyKey parameter introduced in 4.36.0 breaks existing calls to accounting api about xero-node HOT 10 CLOSED

Hi all, Apologies for the delay. We have fixed the idempotency_key parameter issue in the SDK version 5.0.0.

The idempotency_key is now the last optional parameter in the method definitions
Latest release also includes other changes (check out the release notes for details).

Do revert back to us if you are still facing any issues.

Thank you for your patience!

from xero-node.

PETOSS-345

from xero-node.

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

from xero-node.

We have run into same problem. We attempted to use null/undefined for the idempotencyKey parameter but that fails as it seems to be casting the value to a string regardless of what is passed in.

from xero-node.

At first, I tried to proactively fix this issue, as I thought it was a small bug. The specific solution is mentioned in #651.

After analyzing the code, I discovered that the root cause of this issue is:

• 1 The newly added idempotencyKey parameter in version 4.36.0 is an optional parameter, which will be added to the header when sending an HTTP request. It seems that Xero's API does not handle the case when 'Idempotency-Key' is undefined or null, as described in #649 (comment).
• 2 In version 4.36.0, in the automatically generated code by openApi generator, the idempotencyKey is not uniformly configured as an optional parameter in function calls, as described in the issue. This has caused issues with matching the function call parameters when clients upgraded from version 4.35.0 to 4.36.0.

rootCause 1

At first, I thought that root cause 1 would be relatively easy to fix. If we were to directly modify the code of the node version, it would be simple (though not advisable), just like how I fixed it in the closed PR: #651. All we need to do is:

localVarHeaderParams['Idempotency-Key'] = ObjectSerializer.serialize(idempotencyKey, "string");

just change it to the following:

if( idempotencyKey ){
    localVarHeaderParams['Idempotency-Key'] = ObjectSerializer.serialize(idempotencyKey, "string");
}

However, upon closer examination of the code, I realized that it was generated using openApi generator, which complicated things.

I attempted to achieve this by modifying the definition of idempotencyKey in the main project, located at https://github.com/XeroAPI/Xero-OpenAPI.

Definition in version 4.36.0:

    idempotencyKey:
      in: header
      name: Idempotency-Key
      x-snake: idempotency_key
      description: This allows you to safely retry requests without the risk of duplicate processing. 128 character max.
      example: "KEY_VALUE"
      schema:
        type: string

But no matter whether I try to add nullable as false or true, I can't make the generated code change.

if( idempotencyKey ){
    localVarHeaderParams['Idempotency-Key'] = ObjectSerializer.serialize(idempotencyKey, "string");
}

I realized that it is impossible to achieve this by modifying the definition of the YAML file, and it may require modifying some template files mustache.

Therefore, my suggestion is to fix the backend API of Xero so that the 'Idempotency-Key' in the http header can still function properly even if it is null/undefined. This can be resolved without having to upgrade the version of xero-node SDK, which will solve rootCause 1.

rootCause 2

In the xero-node SDK, some methods have added an optional parameter called 'idempotencyKey,' which disrupts the calling code of the original SDK, such as the updateOrCreateInvoices function.

The updateOrCreateInvoices method in version 4.35.0 supports five parameters, which are as follows:

updateOrCreateInvoices (xeroTenantId: string, invoices: Invoices,  summarizeErrors?: boolean, unitdp?: number, options: {headers: {[name: string]: string}} = {headers: {}})

The method for version 4.36.0 has been changed to:

updateOrCreateInvoices (xeroTenantId: string, invoices: Invoices, idempotencyKey?: string, summarizeErrors?: boolean, unitdp?: number, options: {headers: {[name: string]: string}} = {headers: {}})

This causes the calling method of version 4.35.0 to recognize "summarizeErrors" as "idempotencyKey" in version 4.36.0. This results in a breaking change.

Since this code is generated by the OpenAPI generator, we need you to consider whether the current way of calling optional parameters is appropriate and if it needs to be changed in the next version.

updateOrCreateInvoices (xeroTenantId: string, config: {
    invoices: Invoices;
    idempotencyKey?: string;
    summarizeErrors?: boolean;
    unitdp?: number;
}, options: {headers: {[name: string]: string}} = {headers: {}})

The order and number of parameters like this will not cause a break change.

from xero-node.

@itmad-com-au @khross

I just fix this issue manually updated the code in https://github.com/madison890000/xero-node.

Here is the npm link I published: https://www.npmjs.com/package/fix-xero-node.

You can use it by replacing

"xero-node": "4.35.0"

to

"fix-xero-node": "4.36.1"

to upgrading you dependencies of xero-node from 4.35.0 to 4.36.0.

I have only made two changes:

• first is to move the new parameter "idempotencyKey" to the second-to-last position, just before the headers parameter.
  this means no changes need to be done when updating from 4.35.0 to 4.36.0.
• second is deleting the Idempotency-Key in header if it is null/undefined.
  this means no need to add idempotencyKey if you don't want to. and it will ignore it as well if it is set to be null/undefined.

If the official updates to a new version and fixes this bug, you can switch back 'xero-node' again.

from xero-node.

I have a similar issue when using createInvoiceAttachmentByFileName, it now randomly fails with Idempotency Key: [object Object] is used with a different request..

I assume it tries to serialize the latest options object as a string and using it as idempotency key in the request. First request succeeds because it's using [object Object] as idempotency key.

I'm using the same options as the sample from the README for this function, with latest release 4.36.0: https://github.com/XeroAPI/xero-node#accounting-api

I will try to downgrade for now until this is fixed.

Thanks!

from xero-node.

I have a similar issue when using createInvoiceAttachmentByFileName, it now randomly fails with Idempotency Key: [object Object] is used with a different request..

I assume it tries to serialize the latest options object as a string and using it as idempotency key in the request. First request succeeds because it's using [object Object] as idempotency key.

I'm using the same options as the sample from the README for this function, with latest release 4.36.0: https://github.com/XeroAPI/xero-node#accounting-api

I will try to downgrade for now until this is fixed.

Thanks!

This is the definition of createInvoiceAttachmentByFileName.

public async createInvoiceAttachmentByFileName (xeroTenantId: string, invoiceID: string, fileName: string, body: fs.ReadStream, idempotencyKey?: string, includeOnline?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: Attachments;  }> {
        const localVarPath = this.basePath + '/Invoices/{InvoiceID}/Attachments/{FileName}'
        ...

The example in readme is incorrect.
You need change to blow to make it works in 4.36.0.

const accountAttachmentsResponse = await xero.accountingApi.createInvoiceAttachmentByFileName(activeTenantId, invoiceId, filename, readStream,idempotencyKey,true, {
  headers: {
    'Content-Type': contentType
  }
});

from xero-node.

I have a similar issue when using createInvoiceAttachmentByFileName, it now randomly fails with Idempotency Key: [object Object] is used with a different request..

I assume it tries to serialize the latest options object as a string and using it as idempotency key in the request. First request succeeds because it's using [object Object] as idempotency key.

I'm using the same options as the sample from the README for this function, with latest release 4.36.0: https://github.com/XeroAPI/xero-node#accounting-api

I will try to downgrade for now until this is fixed.

Thanks!

This is the definition of createInvoiceAttachmentByFileName.

public async createInvoiceAttachmentByFileName (xeroTenantId: string, invoiceID: string, fileName: string, body: fs.ReadStream, idempotencyKey?: string, includeOnline?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: Attachments;  }> {

        const localVarPath = this.basePath + '/Invoices/{InvoiceID}/Attachments/{FileName}'

        ...

The example in readme is incorrect.

You need change to blow to make it works in 4.36.0.

const accountAttachmentsResponse = await xero.accountingApi.createInvoiceAttachmentByFileName(activeTenantId, invoiceId, filename, readStream,idempotencyKey,true, {

  headers: {

    'Content-Type': contentType

  }

});

Thanks for the info, we will use TS for this part of our code in the future that will avoid future issues like that with Xero.

The problems is see here:

• Why breaking changes were introduced as a minor version release?
• Why the docs / samples are not up to date for that release.

I guess that I will need to be more careful with Xero updates in the future 😬

from xero-node.

I have a similar issue when using createInvoiceAttachmentByFileName, it now randomly fails with Idempotency Key: [object Object] is used with a different request..

I assume it tries to serialize the latest options object as a string and using it as idempotency key in the request. First request succeeds because it's using [object Object] as idempotency key.

I'm using the same options as the sample from the README for this function, with latest release 4.36.0: https://github.com/XeroAPI/xero-node#accounting-api

I will try to downgrade for now until this is fixed.

Thanks!

This is the definition of createInvoiceAttachmentByFileName.

public async createInvoiceAttachmentByFileName (xeroTenantId: string, invoiceID: string, fileName: string, body: fs.ReadStream, idempotencyKey?: string, includeOnline?: boolean, options: {headers: {[name: string]: string}} = {headers: {}}) : Promise<{ response: http.IncomingMessage; body: Attachments;  }> {

        const localVarPath = this.basePath + '/Invoices/{InvoiceID}/Attachments/{FileName}'

        ...

The example in readme is incorrect.
You need change to blow to make it works in 4.36.0.

const accountAttachmentsResponse = await xero.accountingApi.createInvoiceAttachmentByFileName(activeTenantId, invoiceId, filename, readStream,idempotencyKey,true, {

  headers: {

    'Content-Type': contentType

  }

});

Thanks for the info, we will use TS for this part of our code in the future that will avoid future issues like that with Xero.

The problems is see here:

• Why breaking changes were introduced as a minor version release?
• Why the docs / samples are not up to date for that release.

I guess that I will need to be more careful with Xero updates in the future 😬

Yeah, and I don't know why.

if you are have issues when you trying to upgrade from 4.35.0 to 4.36.0 to avoid breaking changes, you can try this:
#649 (comment)

from xero-node.