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

Enhancement: Enable use of Readable stream when uploading attachments #631

Closed
csharpsi opened this issue Feb 16, 2023 · 5 comments
Closed
Assignees

Comments

@csharpsi
Copy link

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

  • Version 4.17.0

Is your feature request related to a problem? Please describe.
Currently, the accounting API allows for attaching documents by using a file stream. This relies on the file existing on disk in order to use the fs.ReadStream API.

Because of this restriction, when auto generating a document receipt in memory, it is necessary to write the file to disk before we can use the fs.createReadStream(...) method to pass the file onto the Xero Accounting API client (in my specific case the updateInvoiceAttachmentByFileName method).

Describe the solution you'd like
The readStream argument uses the type fs.ReadStream. I propose that we define this argument as a union type of fs.ReadStream | Readable | Buffer or define an overloaded method (to maintain backward compatibility) and use the Stream API to enable the use of Readable streams. An example of this would look like:

import { Readable } from "stream";
...

updateInvoiceAttachmentByFileName(xeroTenantId: string, invoiceID: string, fileName: string, body: fs.ReadStream | Readable | Buffer, options?: {
    headers: {
        [name: string]: string;
    };
}): Promise<{
    response: http.IncomingMessage;
    body: Attachments;
}>;

Describe alternatives you've considered
The only alternative currently is to temporarily write the buffer to disk before reading it into an FS stream, which is pretty inefficient.

Additional Context
I'd be happy to submit a PR for this if someone could point me in the right direction. It doesn't look super clear how this would be done using the Open API generator

@github-actions
Copy link

PETOSS-272

@github-actions
Copy link

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

@mikemklee
Copy link

mikemklee commented Dec 8, 2023

Any updates on this? Is the approach described above to temporarily write the buffer to disk before reading it again as a FS stream still the only workaround?

@sangeet-joy-tw
Copy link
Contributor

The changes to accept Buffer & Readable stream type has been released with the new version v5.1.0

Please let us know with any more issues.

@mikemklee @csharpsi

@csharpsi
Copy link
Author

Hi @sangeet-joy-tw

Thanks for the update! However, I don't think it's working.

When I perform an export, the file is not attached to the invoice. I have a unit test that proves that the document stream passed into the updateInvoiceAttachmentByFileName function matches the byte array of the test file and that is passing, so I am confident that the issue is with the SDK.

Example effected invoices:

  • 22e3d6d4-3442-444b-b109-fa359bcacef6
  • 3c425103-1d67-4189-9b1f-3ef1baf286e1

The below examples were imported into Xero, the first using the latest version of the xero-node SDK and the second using version 4.17.0

Latest Version

Version 4.17.0

Additionally, I am seeing a console.log message JSON parse body failed which looks to be coming from the SDK here

image

If I can help any further please let me know

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

3 participants