search

jaydenseric / graphql-upload

Middleware and a scalar Upload to add support for GraphQL multipart requests (file uploads via queries and mutations) to various Node.js GraphQL servers.
https://npm.im/graphql-upload
MIT License
1.43k stars 132 forks source link

Pending promises issue #118

Closed tlupo closed 6 years ago

tlupo commented 6 years ago

Hi @jaydenseric !

First of all thank you very much for this package !

I’m really sorry to talk, like other issues, about apollo-server while your project is now independent but I think that your recent work on graphql-upload can help me to understand my issue.

I have a GraphQL mutation which accepts, as parameter, an array of GraphQLUpload objects. It works most of the time (~95%) but sometimes one of my GraphQLUpload promise stays in pending state indefinitely. I have tried many solutions, like in your example project or in this issue https://github.com/jaydenseric/graphql-upload/issues/67 (with an array of promises and a Promise.all call) but it doesn’t work perfectly. My images are very small (around 20 ko).

I saw in one of your comment that many issues have been fixed in recent versions and apollo-server still uses v5. Do you remember / think, this « pending promise issue » is one of the issue fixed recently in your package or do you have any idea to help me with this ? I don’t know if it’s gonna fix my issue because I use apollo-server and it would be very difficult to stop using but it could help me to understand this issue.

Again, thank you very much for your help and your work. Best regards, Thomas

jaydenseric commented 6 years ago

Sorry for the weekend delay :)

Without code examples it is hard to to exactly where the problem is. In creating tests and fixing bugs there were a few times we encountered file upload promises not resolving, which I can't remember of the top of my head.

If you were to try your resolver with the current graphql-upload version you would likely either see an error or not get the issue.

Chances are that if you work out what is wrong, you might be able to rewrite to avoid this particular issue, but there are other issues with the old versions that no amount of rewriting your resolver code can fix. I wouldn't waste time with the old version personally.

If you would like to share some of your code in the Apollo Slack team #uploads channel I would be happy to take a quick look and see if anything jumps out.

mike-marcacci commented 6 years ago

@tlupo this is certainly possible, but as @jaydenseric mentioned really hard to diagnose without any code.

One huge change addressed in more recent versions is that a multipart request is a single stream which was broken up into multiple individual streams (one for each file). However, these had to be consumed in the same order as they appeared in the initial request, or it would hang indefinitely. In modern versions, the request stream buffers to the file system to allow out-of-order reads, etc. While I don’t really have a diagnosis for you, updating to one of the more recent versions is going to be your first step any way you look at it