When file transforms race and lose, accept defeat gracefully
ClosedPublic
Actions

Authored by epriestley on May 21 2015, 2:50 PM.

Details

Reviewers

btrahan

Maniphest Tasks

Commits

Summary

Fixes T8277. Transforming files can race; resolve the race after we lose.

Test Plan

Added sleep(10) near the bottom of the transform controller.
Transformed a file in two browser windows at the same time; got something like this (exception corresponds to the loser of the race):

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

epriestley retitled this revision from to When file transforms race and lose, accept defeat gracefully.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

btrahan edited edge metadata.

btrahan added inline comments.

src/applications/files/controller/PhabricatorFileTransformController.php
90	Is this more like a 500?

This revision is now accepted and ready to land.May 21 2015, 4:30 PM

src/applications/files/controller/PhabricatorFileTransformController.php
90	My thinking on 404 is that we "should" only get here if the thing was just deleted, and we'd 404 if we were a fraction of a second quicker (on line 105, or after redirecting on line 111), and 404'ing in those cases makes fairly good sense to me. In theory, we don't need to 404 here and could just loop back to the top and rebuild the transform, or loop here trying to either save our own transform or load a conflicting one, but those are both complicated and introduce the possibility that we might loop forever if there's a bug somewhere.