Skip to content

Conversation

@Lukas-Kullmann
Copy link

@Lukas-Kullmann Lukas-Kullmann commented Jul 10, 2019

Description

It basically fixes the problem reported in this comment.

The problem is, that streams are serialized by the swagger client to a string (via JSON.stringify), which should not be done. They should be passed on to the FormData as is.

How Has This Been Tested?

I tested uploading files via stream. Before it did not work, after it did.

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

I'm not sure about the breaking change. I removed the Buffer handling from the code. But it was not working anyway. So I'm not sure if you want to treat this as breaking.

The problem is, that for buffers, a file name has to be set since it cannot be inferred by FormData. (See form-data's alternative submission methods section):

Form-Data can recognize and fetch all the required information from common types of streams (fs.readStream, http.response and mikeal's request), for some other types of streams you'd need to provide "file"-related information manually

The problem is, that we cannot just set buffer or some constant. The server can then not infer the file type since there is no mime type and no file extension for the file.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I'm not sure about code style. The linter does not compalin, but I don't know if you have any additional code style rules.

@tim-lai
Copy link
Contributor

tim-lai commented May 13, 2020

@Lukas-Kullmann Thanks for submitting this PR. The root cause of referenced issue is fixed with #1527. The use of streams is for tests, or in non-browser environments. Update: if you are still experiencing issues in a node.js environment after the next release of swagger-js, please feel free to re-open this issue.

@tim-lai tim-lai closed this May 13, 2020
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

Successfully merging this pull request may close these issues.

5 participants