flysystem icon indicating copy to clipboard operation
flysystem copied to clipboard

Should exceptions include prefixed source/destination paths?

Open ziadoz opened this issue 10 months ago • 0 comments

Question

We're using Flysystem via Laravel (11.36.1), and a silly bug slipped into our codebase where we accidentally passed absolute paths to the move() method instead of relative ones.

This caused the underlying LocalFilesystemAdapter to throw an UnableToMoveFile exception, however the paths in the exception message aren't prefixed, which made the bug very difficult to discover.

The code is fairly simple:

// config/filesystems.php
/*
'foobar' => [
      'driver'      => 'local',
      'root'        => storage_path('filesystems/local/foobar'),
      'throw'       => true,
  ],
*/

// Our broken code...
Storage::disk('foobar')->move(
    '/srv/app/storage/filesystems/foobar/folder/foo.txt', 
    '/srv/app/storage/filesystems/foobar/folder/bar.txt',
);

This caused an exception:

  "exception": {
    "class": "League\\Flysystem\\UnableToMoveFile",
    "code": 0,
    "file": "/srv/app/vendor/league/flysystem/src/UnableToMoveFile.php:56",
    "message": "Unable to move file from srv/app/storage/filesystems/foobar/folder/foo.txt to srv/app/storage/filesystems/foobar/folder/bar.txt, because unknown reason",
    "trace": [...],
}

I suppose the lack of a / at the beginning of the filename possibly should have tipped us off, but at a glance the filenames in the exception looked fine without further digging.

It's only when I dug into the move() method and dumped out the $sourcePath and $destinationPath variables that I realised we were actually trying to move /srv/app/storage/filesystems/foobar/srv/app/storage/filesystems/foobar/folder/foo.txt to /srv/app/storage/filesystems/foobar/srv/app/storage/filesystems/foobar/folder/bar.txt, and Flysystem was unable to create all the sub-directories due to the weird paths. https://github.com/thephpleague/flysystem/blob/3.x/src/Local/LocalFilesystemAdapter.php#L256

Should the exceptions thrown by Flysystem include the prefixed source/destination paths? It seems like it could possibly give a more accurate idea of what operation was occurring when the error happened, which would be useful when debugging.

ziadoz avatar Dec 19 '24 15:12 ziadoz