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

Files.getReadStream() Doesn't Return File Data for FormData Request #801

Closed
4 tasks done
noseworthy opened this issue Feb 6, 2023 · 8 comments
Closed
4 tasks done

Comments

@noseworthy
Copy link

Description of the Issue

A recent bug fix has caused an issue in our app. #790 fixed Files.getReadStream() so that it wraps the read stream in a PassThrough to prevent issues when data isn't immediately read from the stream that is returned. However, this breaks compatibility with the form-data library that we were using to pipe the file contents via multipart/form-data request to another API.

import fetch from 'node-fetch';
import FormData from 'form-data';
import BoxClient from 'box-node-sdk/lib/box-client';

async function myFunc(client: BoxClient, fileId: number) {
  const stream = await client.files.getReadStream(fileId);

  const formData = new FormData();
  form.append('file', stream);

  const response = await fetch(
    'https://example.com/file-upload',
    {
      method: 'POST',
      body: formData
    });

    return response.json();
}

Unfortunately we don't control the other API and so we have to use multipart/form-data. Previously I was able to just pass the stream directly, but now after #790 I have to manually provide the file data as described in the form-data documentation about only supporting fs.readStream like so:

import fetch from 'node-fetch';
import FormData from 'form-data';
import mime from 'mime-types';

import BoxClient from 'box-node-sdk/lib/box-client';

async function myFunc(client: BoxClient, fileId: number) {
  const [fileData, stream] = await Promise.all([
    client.files.get(fileId),
    client.files.getReadStream(fileId),
  ]);

  const formData = new FormData();
  form.append('file', stream, {
    filename: fileData.name,
    contentType: mime.lookup(fileData.name) || undefined,
    knownLength: fileData.size,
  });

  const response = await fetch(
    'https://example.com/file-upload',
    {
      method: 'POST',
      body: formData
    });

    return response.json();
}

It'd be nice if this still worked without me having to look up the file data.

Steps to Reproduce

Unfortunately you'll need an API endpoint that accepts multipart/form-data POST requests to try this, but if you call the function provided below, you should see some errors where the form data isn't delivered properly to the endpoint.

import fetch from 'node-fetch';
import FormData from 'form-data';
import BoxClient from 'box-node-sdk/lib/box-client';

async function myFunc(client: BoxClient, fileId: number) {
  const stream = await client.files.getReadStream(fileId);

  const formData = new FormData();
  form.append('file', stream);

  const response = await fetch(
    'https://example.com/file-upload',
    {
      method: 'POST',
      body: formData
    });

    return response.json();
}

Expected Behavior

I expected the PassThrough stream to contain the exact same information as the original read stream returned from this method, including the file information that form-data needs.

Error Message, Including Stack Trace

I don't really have a stack trace, the API I use doesn't log anything and just returns an empty response if there's any issues. This was manifesting in our app as getting an empty response from the API that we were unable to JSONify.

Versions Used

Node SDK: 2.8.1

@noseworthy
Copy link
Author

I think this is where they're doing the magic: https://github.com/form-data/form-data/blob/master/lib/form_data.js#L234

I'm not sure if it's even possible to fix this. Just figured I'd let you folks know.

@congminh1254
Copy link
Member

Hi @noseworthy,

Thanks for submitting this issue, we are checking it and will get back to you ASAP.

Thanks for your patience.

Minh

@congminh1254
Copy link
Member

Hi @noseworthy,

Can you try to follow this instruction and let us know if it works?

https://stackoverflow.com/questions/69315652/how-to-pass-duplex-stream-in-formdata

Thank you.

@noseworthy
Copy link
Author

Hi @noseworthy,

Can you try to follow this instruction and let us know if it works?

https://stackoverflow.com/questions/69315652/how-to-pass-duplex-stream-in-formdata

Thank you.

Hey @congminh1254, thanks for looking at my issue, and for digging up that stackoverflow post! Yeah, this is the workaround we used in our app to get around this issue. I show how we did it up in the description. I use the files.get() api to get the file name so that I can provide that and the mimetype to form-data.

So yeah, this approach works. I mostly wanted to open this issue to let you know that the PassThrough approach changes how the stream returned from this API behaves with things like form-data. I'm not sure if that's something you can address - or even want to - but I figured I'd bring it up.

@congminh1254
Copy link
Member

Hi @noseworthy,

I'm sorry for misunderstanding :D we will discuss with our team to see if it's something need to change or it will be keep for the future if there is no other solution.

We will let you know when we have the answer.

Best regards,
Minh

@stale
Copy link

stale bot commented Mar 9, 2023

This issue has been automatically marked as stale because it has not been updated in the last 30 days. It will be closed if no further activity occurs within the next 7 days. Feel free to reach out or mention Box SDK team member for further help and resources if they are needed.

@stale stale bot added the stale label Mar 9, 2023
@stale
Copy link

stale bot commented Mar 16, 2023

This issue has been automatically closed due to maximum period of being stale. Thank you for your contribution to Box Node SDK and feel free to open another PR/issue at any time.

@stale stale bot closed this as completed Mar 16, 2023
@mgrytsai mgrytsai added dontstale and removed stale labels Mar 16, 2023
@mgrytsai mgrytsai reopened this Mar 16, 2023
@congminh1254
Copy link
Member

Hi @noseworthy

Sorry for the late response, we have just added a configuration to disable PassThrough for the streaming response. It may fix this issue. Please check the document here.

Best,
Minh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants