Rescue FileNotFoundError exception on failed image downloads #4026
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
|
Failed on Paperclip |
kennyadsl
reviewed
Apr 14, 2021
| end | ||
|
|
||
| click_button "Update" | ||
| Spree::Image.first.attachment.blob.update(key: 11) |
There was a problem hiding this comment.
Specs are failing in the CI in jobs where we do not use Active Storage but Paperclip. In that case, attachment doesn't respond to blob and it fails.
I think we need to find a way to make this compatible with both or skip the test when we are using Paperclip.
There was a problem hiding this comment.
Skipping test based on which module is used.
kennyadsl
added
changelog:solidus_core
In rare circumstances, an image would fail to upload to a third party storage service, causing the application to hard crash when trying to retrieve an image resource. This commit implements a fix to return a place holder image to alert the user of an error and also prevents the application from crashing. This fix will not, however, save any errors associated with the failed upload such as credential errors. This fix was tested by artificially changing the blob record to force a FileNotFoundError. Commit History: Implemented fix for AS::FileNotFound Error" Implemented feature test for potential FileNotFoundError Added conditional to test because it is specific to Active Storage
cpfergus1
force-pushed
the
Issue-4017-FileNotFound
branch
from
da8ffac to
af78b4b
Compare
kennyadsl
approved these changes
Apr 14, 2021
elia
approved these changes
Apr 14, 2021
jarednorman
approved these changes
Apr 14, 2021
kennyadsl
changed the title
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 15, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record. This compliments [PR solidusio#4026](solidusio#4026) which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 15, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments [PR solidusio#4026](solidusio#4026) which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata. - metadata returns hash with strings, not symbols caused nil values to be returned regardless if the image was good or not.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 15, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata. - metadata returns hash with strings, not symbols caused nil values to be returned regardless if the image was good or not.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 15, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata. - metadata returns hash with strings, not symbols. It appeared that this caused nil values to be returned regardless if the image was good or not.
5 tasks
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 15, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata. - metadata returns hash with strings, not symbols. It appeared that this caused nil values to be returned regardless if the image was good or not.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 16, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata. - metadata returns hash with strings, not symbols. It appeared that this caused nil values to be returned regardless if the image was good or not.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Jun 16, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Sep 13, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Sep 13, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Sep 14, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Sep 14, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Sep 14, 2021
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 25, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
cpfergus1
added a commit
to cpfergus1/solidus
that referenced
this pull request
Aug 26, 2022
This commit adds the ability for the application to catch missing images with Active Storage during an API call should a file become corrupt or deleted without removing the record through appropriate channels. This compliments PR solidusio#4026 which prevented the same error on the backend. The change will return and image with attributes equal to nil and noimage replacements if an image cannot be sourced. Reasons for the changes: - image.attachment.url bypasses rescue check found in ActiveStorageAdapter - API calls hit height and width prior to url rescue, had to implement rescue in Attachment#metadata.
7 tasks
This was referenced Dec 1, 2022
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.
Description
This PR aimed to solve the Active::Storage::FileNotFoundError bug described in Issue #4017 by to handling FileNotFound error when ActiveStorage fails to download from a remote service such as S3.
In rare circumstances, an image would fail to upload to a third party storage service, causing the application to hard crash when trying to retrieve an image resource. This commit implements a fix to return a place holder image to alert the user of an error and also prevents the application from crashing. This fix will not, however, save any errors associated with the failed upload such as credential errors. This fix was tested by artificially changing the blob record to force
a FileNotFoundError.
The handling of the FileNotFound error maybe a temporary fix as we are aiming to discover another pathway to handle the exception and remove the invalid objects simultaneously.
Checklist: