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

fix: move parser's resolve() from field to finish event #385

Closed
wants to merge 1 commit into from

Conversation

heyask
Copy link

@heyask heyask commented Mar 23, 2024

Fix #384

We're encountering a situation where only the first GraphQLUpload argument in a GraphQL request gets processed. Subsequent arguments result in empty uploads. This behavior seems to be linked to the busboy parser's field event being triggered multiple times, corresponding to the number of GraphQLUpload arguments present.

To address this, we've moved the promise resolution to the finish event. This ensures that the promise only resolves after all files have been processed, successfully capturing all uploads in the request.

@heyask heyask marked this pull request as draft March 23, 2024 08:36
@heyask heyask marked this pull request as ready for review March 23, 2024 08:36
@heyask heyask changed the title fix: move parser\'s resolve() from field to finish event fix: move parser's resolve() from field to finish event Mar 23, 2024
@jaydenseric
Copy link
Owner

This change is not correct; what it does is causes the entire multipart request stream to finish uploading before resolving the operations. By design the operations is supposed to resolve before the files start streaming in, so that way GraphQL resolvers can decide how to handle the file upload streams as they come in.

You can see a graphic showing how this approach makes the overall timings for a GraphQL response better here:

https://github.com/jaydenseric/graphql-multipart-request-spec/blob/master/readme.md

As per #384 (comment) , make sure that you are consuming readable streams for every upload in resolvers.

@jaydenseric jaydenseric closed this Apr 7, 2024
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.

upload multiple images has error
2 participants