Fix #4914: RuntimeError corrupting uploads

Bug: If a media asset got stuck in the 'processing' state during upload,
then it would stay stuck forever and the file couldn't be uploaded again
later.

Fix: Mark stuck assets as failed before raising the "Upload failed"
error. Once the asset is marked as failed, it can be uploaded again
later. Also, only wait for assets to finish processing if they were
uploaded less than 5 minutes ago. If a processing asset is more than 5
minutes old, consider it stuck and mark it as failed immediately.

Assets getting stuck in the processing state is a 'this should never
happen' error. Normally if any kind of exception is raised while
uploading the asset, the asset will be set to the 'failed' state. The
only way an asset can get stuck is if it fails and the exception handler
doesn't run, or the exception handler itself fails. This might happen if
the process is unexpectedly killed, or possibly if the HTTP request
times out and a TimeoutError is raised at an inopportune time. See below
for discussion of issues with Timeout.

[1]: https://vaneyckt.io/posts/the_disaster_that_is_rubys_timeout_method/
[2]: https://jvns.ca/blog/2015/11/27/why-rubys-timeout-is-dangerous-and-thread-dot-raise-is-terrifying/
[3]: https://adamhooper.medium.com/in-ruby-dont-use-timeout-77d9d4e5a001
[4]: https://ruby-doc.org/core-3.0.2/Thread.html#method-c-handle_interrupt-label-Guarding+from+Timeout-3A-3AError
This commit is contained in:
evazion
2021-11-08 17:40:19 -06:00
parent 2225c9b472
commit 8f36ebe2b8
2 changed files with 32 additions and 4 deletions

View File

@@ -284,6 +284,20 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest
end
end
context "when re-uploading a media asset stuck in the 'processing' state" do
should "mark the asset as failed" do
asset = create(:media_asset, file: File.open("test/files/test.jpg"), status: "processing")
file = Rack::Test::UploadedFile.new("test/files/test.jpg")
post_auth uploads_path, @user, params: { upload: { file: file, tag_string: "abc", rating: "e", }}
upload = Upload.last
assert_redirected_to upload
assert_match("MediaAsset::Error", upload.reload.status)
assert_equal("failed", asset.reload.status)
end
end
context "uploading a file from your computer" do
should_upload_successfully("test/files/test.jpg")
should_upload_successfully("test/files/test.png")