fix: Get product image uploads working again #6309
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Fixes #6298 mostly by updating to dependencies with the fixes - @reactioncommerce/[email protected] - @reactioncommerce/[email protected] - @reactioncommerce/[email protected] - Also switch to the node v14.15.0 docker-base images - Unclear if this is strickly required but yet again the sharp package native add-on compiling was failing with C dependency errors Signed-off-by: Peter Lyons <[email protected]>
|
Ah, I now see that DCO wants mailchimp email addresses. OK fixed that. |
|
@mikemurray Do you know if this is CircleCI being slow? |
|
I am now able to upload an initial image with this PR, however, I am still running into an error when trying to duplicate a product that has an image on it. It seems related, but this is a good first step: #6310 |
|
Yup, seems like it. Ran test locally and they all completed successfully. Also tested uploading images and works! Great job! |
- fixes #6310 Signed-off-by: Peter Lyons <[email protected]>
|
I added another commit that hopefully will fix #6310 too. Tomorrow will look into CI timing out with mongo errors. |
|
@focusaurus I have verified that duplicating variants with images works as expected. |
Signed-off-by: Peter Lyons <[email protected]>
|
CI is fixed with a new release of api-core. |
kieckhafer
approved these changes
Nov 18, 2020
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the sharp package native add-on compiling was failing with
C dependency errors
Resolves #6298
Impact: minor
Type: bugfix
Issue
Uploading product images from within reaction-admin was failing consistently with a chain of errors originating deep within our file upload stack (file-collections, the upload server, and supporting code). See #6298 for more details but ultimately there was a nodejs stream object representing the write to gridfs, and code was calling
stream.destroy(). The normal behavior for that is to emit an"error"event with no arguments. I believe that bit of code is functioning as designed. In the reaction code, we bound event listeners to this event but failed to handle the case when this event had to passed arguments.Solution
The fixes amounted to detecting these spurious error events and ignoring them, allowing things to proceed successfully.
Breaking changes
None expected.
Testing
I've tested top-level product images and variants and they work.