Contrib.social

Log SymfonyFileException errors on file_save_upload

Open on Drupal.org →
Created on 22 June 2023, almost 2 years ago

Problem/Motivation

Recently, when trying to upload files on a new environment (Using the IMCE → module), a user was getting the following error:

The file could not be saved. An unknown error has occurred.

There was nothing in the Apache or Drupal logs, the AJAX request for upload returned a 200 HTTP response (that printed the error to the page), so there was seemingly no way to know what was happening exactly.

Looking through the code, the error message was being sent in file.module's file_save_upload() function:

catch (SymfonyFileException $e) {
      \Drupal::messenger()->addError(t('The file %file could not be saved. An unknown error has occurred.', ['%file' => $uploaded_file->getFilename()]));
      $files[$i] = FALSE;
    }

So, when catching the generic file exception from symfony, we are only informing that an unknown error occured. But the exception message would prove to be way more useful. In our case, if we tried to log the exception message:

catch (SymfonyFileException $e) {
      \Drupal::messenger()->addError(t('The file %file could not be saved. An unknown error has occurred.', ['%file' => $uploaded_file->getFilename()]));
      \Drupal::logger('file')->notice($e->getMessage());
      $files[$i] = FALSE;
    }

We could see that the error wasn't unknown at all:

Symfony\Component\HttpFoundation\File\Exception\NoTmpDirFileException: File could not be uploaded: missing temporary directory

With that information, we were able to promptly fix it.

I think we shouldn't ignore such an important exception message. In fact, it wouldn't even be unusual to log the messages when handling file exceptions, as further down below that same code, on a different catch statement, we are logging the error:

catch (FileWriteException $e) {
      \Drupal::messenger()->addError(t('File upload error. Could not move uploaded file.'));
      \Drupal::logger('file')->notice('Upload error. Could not move uploaded file %file to destination %destination.', ['%file' => $uploaded_file->getClientOriginalName(), '%destination' => $destination . '/' . $uploaded_file->getClientOriginalName()]);
      $files[$i] = FALSE;
    }

Seems like this was just an oversight, and we should fix it.

Steps to reproduce

In our case, we can reproduce it like this:

- Install and configure the IMCE module.
- Set the php.ini variable "upload_tmp_dir" to a folder that does not exist.
- Attempt to upload a file through the IMCE module.

Proposed resolution

Log the exception message.

Remaining tasks

Log the exception message.

User interface changes

The log will now appear in the logs (Accessible through the backoffice if database logging is enabled).

API changes

None.

Data model changes

None.

📌 Task
Status

Active

Version

10.1 ✨

Component
File system  →

Last updated 1 day ago

Created by

🇵🇹Portugal gueguerreiro Lisboa

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @gueguerreiro
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update almost 2 years ago
    29,436 pass
  • @gueguerreiro opened merge request.
  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update almost 2 years ago
    29,436 pass
  • Comment almost 2 years ago →
    🇵🇹Portugal gueguerreiro Lisboa

    Opened a PR with the one line code change, but I don't remember if we prefer patches for Core changes, so here's that too.

  • Comment almost 2 years ago →
    🇮🇳India sumit-k

    I have attempted to replicate the issue following the steps provided. However, I have encountered a slight deviation in the behavior.

    When I set the "upload_tmp_dir" variable in the php.ini file to a non-existent folder, instead of throwing a SymfonyFileException as expected, it is displaying an Ajax notice with the message: "Notice: Unknown: file created in the system's temporary directory in Unknown on line 0."

    It is not falling under SymfonyFileException In that case, no log messages are getting logged.
    I would appreciate it if you could provide me with any additional steps or considerations that I may have missed during replication.

  • Status changed to Postponed: needs info almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States smustgrave

    Needs additional steps per #4

  • Status changed to Closed: outdated 15 days ago
  • Comment 15 days ago →
    🇺🇸United States smustgrave

    Since no steps provided in almost 2 years going to close out as outdated, if still relevant for D11 please re-open!

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024