Skip to content

Commit 7358478

Browse files
author
epriestley
committed
Improve error messages when hitting PHP file upload issues
Summary: See T429. When you hit certain errors, you get less-than-helpful messages like "upload error 3". Instead, produce human-readable errors. Test Plan: Simulated errors, verified user receives decent error messages. Reviewed By: aran Reviewers: jungejason, tuomaspelkonen, aran, startupguy CC: aran Differential Revision: 816
1 parent ec0d91a commit 7358478

File tree

5 files changed

+58
-1
lines changed

5 files changed

+58
-1
lines changed

src/__phutil_library_map__.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@
420420
'PhabricatorFileTransformController' => 'applications/files/controller/transform',
421421
'PhabricatorFileURI' => 'applications/files/uri',
422422
'PhabricatorFileUploadController' => 'applications/files/controller/upload',
423+
'PhabricatorFileUploadException' => 'applications/files/exception/upload',
423424
'PhabricatorFileViewController' => 'applications/files/controller/view',
424425
'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/garbagecollector',
425426
'PhabricatorGoodForNothingWorker' => 'infrastructure/daemon/workers/worker/goodfornothing',
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* Copyright 2011 Facebook, Inc.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
final class PhabricatorFileUploadException extends Exception {
20+
21+
public function __construct($code) {
22+
$map = array(
23+
UPLOAD_ERR_INI_SIZE =>
24+
"Uploaded file is too large: file is larger than the ".
25+
"'upload_max_size' setting in php.ini.",
26+
UPLOAD_ERR_FORM_SIZE =>
27+
"File is too large.",
28+
UPLOAD_ERR_PARTIAL =>
29+
"File was only partially transferred, upload did not complete.",
30+
UPLOAD_ERR_NO_FILE =>
31+
"No file was uploaded.",
32+
UPLOAD_ERR_NO_TMP_DIR =>
33+
"Unable to write file: temporary directory does not exist.",
34+
UPLOAD_ERR_CANT_WRITE =>
35+
"Unable to write file: failed to write to temporary directory.",
36+
UPLOAD_ERR_EXTENSION =>
37+
"Unable to upload: a PHP extension stopped the upload.",
38+
);
39+
40+
$message = idx($map, $code, "Upload failed: unknown error.");
41+
parent::__construct($message, $code);
42+
}
43+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
/**
3+
* This file is automatically generated. Lint this module to rebuild it.
4+
* @generated
5+
*/
6+
7+
8+
9+
phutil_require_module('phutil', 'utils');
10+
11+
12+
phutil_require_source('PhabricatorFileUploadException.php');

src/applications/files/storage/file/PhabricatorFile.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static function newFromPHPUpload($spec, array $params = array()) {
4848

4949
$err = idx($spec, 'error');
5050
if ($err) {
51-
throw new Exception("File upload failed with error '{$err}'.");
51+
throw new PhabricatorFileUploadException($err);
5252
}
5353

5454
$tmp_name = idx($spec, 'tmp_name');

src/applications/files/storage/file/__init__.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77

88

9+
phutil_require_module('phabricator', 'applications/files/exception/upload');
910
phutil_require_module('phabricator', 'applications/files/storage/base');
1011
phutil_require_module('phabricator', 'applications/files/uri');
1112
phutil_require_module('phabricator', 'applications/phid/constants');

0 commit comments

Comments
 (0)