From 27b51e619237116d99e03fc1b3e8cd6399087214 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 27 Apr 2017 16:17:04 -0700 Subject: [PATCH] Fix two minor issues with "arc download" Summary: Ref T12651. Ran into these during D17799: - Use `getStatusCode()` to put the actual status code into the message. - If we fail but wrote an empty file to reserve the filename, clean it up. Test Plan: - Faked the error, `phlog()`'d the exception. - Saw sensible exception message. - Saw empty file get cleaned up. Reviewers: chad, amckinley Reviewed By: chad Maniphest Tasks: T12651 Differential Revision: https://secure.phabricator.com/D17800 --- src/workflow/ArcanistDownloadWorkflow.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/workflow/ArcanistDownloadWorkflow.php b/src/workflow/ArcanistDownloadWorkflow.php index 66fa7331..543b715e 100644 --- a/src/workflow/ArcanistDownloadWorkflow.php +++ b/src/workflow/ArcanistDownloadWorkflow.php @@ -83,6 +83,7 @@ EOTEXT $display_name = 'F'.$id; $is_show = $this->show; $save_as = $this->saveAs; + $path = null; try { $file = $conduit->callMethodSynchronous( @@ -186,7 +187,7 @@ EOTEXT throw new Exception( pht( 'Got HTTP %d status response, expected HTTP 200.', - $status)); + $status->getStatusCode())); } if (strlen($data)) { @@ -232,6 +233,14 @@ EOTEXT return 0; } catch (Exception $ex) { + + // If we created an empty file, clean it up. + if (!$is_show) { + if ($path !== null) { + Filesystem::remove($path); + } + } + // If we fail for any reason, fall back to the older mechanism using // "file.info" and "file.download". }