laravel / nova-issues

554 stars 34 forks source link

Nova needs better error handling when file uploads fail #5882

Closed connecteev closed 1 year ago

connecteev commented 1 year ago

Description:

Related to the issue described here https://github.com/laravel/nova-issues/issues/5880 Although the original issue (of failing file uploads) was resolved due to max allowable file sizes by PHP / Nginx, Nova needs to handle errors more gracefully.

When file uploads dont work (say, because the max PHP file size limit and/or the max file size specified by Nginx or Herd is exceeded), Nova doesn't seem to know of / handle errors very gracefully. Sometimes, it thinks that the file was uploaded and the resource was created, even though neither of those happened. At other times, it displays a server error.

Screenshots of all scenarios can be found below.

Detailed steps to reproduce the issue on a fresh Nova installation:

Create a CourseVideo Nova resource with the following File field type to accept file / video uploads

File::make('Video Path', 'video_path')
                ->disk('public')
                ->acceptedTypes('video/*')
                ->store(function (Request $request, $model) {
                    return [
                        'video_path' => $request->file('video_path')->store('attachments'),
                    ];
                })
                ->sortable()
                ->required() // alternative to using 'required' in the rules() method
                //->rules('required', 'string', 'max:255')
            ,

course_videos migration file:

    public function up(): void
    {
        Schema::create('course_videos', function (Blueprint $table) {
            $table->id();
            $table->string('title');
            $table->string('slug')->unique();
            $table->text('description');
            $table->string('video_path');
            $table->timestamps();
        });
    }

Set a low value for upload_max_filesize and post_max_size in your php.ini file: /Users/connecteev/Library/Application Support/Herd/config/php/{74,80,81,82,83}/php.ini: Added to all php.ini files:

upload_max_filesize=1M
post_max_size=1M

Set a low value for client_max_body_size in your nginx.conf and/or herd.conf file:

/Users/connecteev/Library/Application Support/Herd/config/nginx/nginx.conf: client_max_body_size 1M;

/Users/connecteev/Library/Application Support/Herd/config/nginx/herd.conf: client_max_body_size 1M;

Test with files > 1MB at https://file-examples.com/index.php/sample-video-files/sample-mov-files-download/ and https://file-examples.com/index.php/sample-video-files/sample-mp4-files/

Now, go to the Nova admin and try to upload a new video, in my case: /admin/resources/course-videos/new On submitting the form (in my case pressing the "Create Course Video" button), you either see:

  1. Case 1: a success message (as shown in the screenshot below) when a small file (< 1MB) is uploaded, there is an entry added to the database and the file upload worked properly
  2. Case 2: Try to upload a large file (> 1MB in this example), you see a success message (as shown in the screenshot below), BUT there is no entry added to the database and the file upload did not work
  3. Case 3: Try to upload a large file (> 1MB in this example), you see the "Server error" and "There was a problem submitting the form" error messages (as shown in the screenshot below)

Case 1 (File upload works successfully):

image

Case 2 (File upload fails, but Nova thinks it worked):

image

Case 3 (File upload fails, Nova gives an error message):

image

In both case 2 and 3, this is what you see a similar error in the laravel.log server logs:

[2023-09-11 15:45:59] local.ERROR: SQLSTATE[HY000]: General error: 1364 Field 'video_path' doesn't have a default value (Connection: mysql, SQL: insert into `course_videos` (`title`, `slug`, `description`, `updated_at`, `created_at`) values (This is a test video, this-is-a-test-video, <div>Testing video uploads in Nova</div>, 2023-09-11 15:45:59, 2023-09-11 15:45:59)) {"userId":1,"exception":"[object] (Illuminate\\Database\\QueryException(code: HY000): SQLSTATE[HY000]: General error: 1364 Field 'video_path' doesn't have a default value (Connection: mysql, SQL: insert into `course_videos` (`title`, `slug`, `description`, `updated_at`, `created_at`) values (This is a test video, this-is-a-test-video, <div>Testing video uploads in Nova</div>, 2023-09-11 15:45:59, 2023-09-11 15:45:59)) at /Code/web/__Herd/inertia_app/vendor/laravel/framework/src/Illuminate/Database/Connection.php:801)
crynobone commented 1 year ago

You can use https://github.com/laravel/framework/blob/10.x/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php Illuminate\Foundation\Http\Middleware\ValidatePostSize to cover this use case.

connecteev commented 1 year ago

Can you share an example of how this would work with Nova? Are you referring to a rule like this? ->rules('required', 'file', 'max:2048000')

Also, what about Case 2, which looks like a bug?

crynobone commented 1 year ago

You can read about middleware using https://laravel.com/docs/10.x/middleware#main-content

Also, required() isn't the same as rules('required'), this has been explicitly mentioned in the documentation.