Corrupted uploads with ActiveStorageAdapter crash the store

[ok32]

I'm having a situation with attachments once again.
I've once opened a discussion about corrupted uploads, and the solution that I offered there has already been PR'd and merged into master.

But now I've encountered a slightly different case. Turns out the user uploaded an image successfully, and then tried to do something with the stock – the URL was /api/stock_locations/1/stock_items/58 and there's StockItemsController in the backtrace. And it caused some exception in ActiveStorage (as seen in the Airbrake error notification email):

ERROR MESSAGE	No such file or directory @ rb_file_s_size - /tmp/image_processing20210608-1-1lsc5ns.jpg

BACKTRACE (a part of)

/usr/local/lib/ruby/2.7.0/tempfile.rb:226 in size
/usr/local/lib/ruby/2.7.0/tempfile.rb:226 in size
/GEM_ROOT/gems/activestorage-6.1.3.2/lib/active_storage/service/s3_service.rb:30 in block in upload

...

/GEM_ROOT/gems/solidus_api-3.0.1/app/controllers/spree/api/stock_items_controller.rb:42 in update

...

And all this resulted in a situation where, again, every page which contains images of this product crashes with an exception (admin pages too). The exception is raised here:

module Spree
  module ActiveStorageAdapter
    # Decorares AtiveStorage attachment to add methods exptected by Solidus'
    # Paperclip-oriented attachment support.
    class Attachment

      # ...

      def variant(style = nil)
        size = style_to_size(style)
        @attachment.variant(
          resize_to_limit: size,
          strip: true
        ).processed # <---- HERE! : "undefined method `processed' for nil:NilClass"
      end

      # ...

    end
  end
end

So @attachment itself is not nil (otherwise .variant() would not have be called in the first place), but .variant() returns nil.

So the question is, how come? What could cause such a corruption? Can it be fixed? Where?

Solidus Version: 3.0.1

To Reproduce
No idea.

Current behavior
Corrupted attachment causes all pages containing it to crash. Impossible to fix from the UI.

Expected behavior
I expect this situation to be admin-recoverable. If something's gone wrong during attachment manipulation the admin should be able to remove/reupload the corrupted one.

[cpfergus1]

Thank you for bringing this up! Is there any information on what the user might be editing through the API? I am looking into reproducing the error.

[ok32]

I think they just adjusted count on hand from the admin UI:

[ok32]

Btw, I think it should be image.url instead of image.attachment.url here to make this fix work in the API views which use this partial.

[cpfergus1]

I have updated pretty much every option I can think of. At the beginning of your post, you mentioned the url was /api/stock_locations/1/stock_items/58 so I sent some patch requests as well and I have not recreated the error yet. Is there anymore information the user might be able to inform us on what specific actions they took? I ran through this on master so I will run through it again on 3.0

[cpfergus1]

Can I please get your Active Storage settings? Any modifications to the module or adapter? What service are you using, local?
Also by chance did this occur during the Fastly crash? https://status.fastly.com/

[ok32]

I'm using a minio running in my own k8s cluster. Not sure if and how it can be related to the Fastly crash, but even if it was, it should not have left the system in such a corrupted state.

My storage config is nothing special:

my_minio:
  public: true
  service: S3
  endpoint: ...
  bucket: ...
  region: "eu-north-1"
  force_path_style: true
  access_key_id: ...
  secret_access_key: ...

[cpfergus1]

Just trying to figure out if there could have been some interruption with data transfer that may have lead to the corruption. Still working on recreating the bug. If you have any other information, please let me know and thank you.

[cpfergus1]

I reached out to @kennyadsl and we agree that the changes you suggested should be made as image.attachment.url bypasses the earlier fix to catch when an attachment image is corrupted.

Btw, I think it should be image.url instead of image.attachment.url here to make this fix work in the API views which use this partial.

Additionally, image.attachment.url is only used in this partial and nowhere else in the application. It most likely should have been implemented as image.url from the beginning.

I will put out a PR for this, however, the patch will most likely not reach the underlying issue of why your image was corrupted in the first place. If you and your team happen to reproduce the bug please let us know (or even breaking the image after the PR). We will keep this issue open for a while.

[waiting-for-dev]

Closed by #4103