D10.3 Drupal\s3fs_streamwrapper\S3fsFileSystemService::prepareDestination(): TypeError: Argument #3 ($replace) must be of type int, Drupal\Core\File\FileExists given

Created on 18 July 2024, 8 months ago
Updated 19 July 2024, 8 months ago

Problem/Motivation

Fix PHP error :

TypeError: Drupal\s3fs_streamwrapper\S3fsFileSystemService::prepareDestination(): Argument #3 ($replace) must be of type int, Drupal\Core\File\FileExists given, called in modules/contrib/s3fs/modules/s3fs_streamwrapper/src/S3fsFileSystemService.php on line 228 in Drupal\s3fs_streamwrapper\S3fsFileSystemService->prepareDestination() (line 302 of modules/contrib/s3fs/modules/s3fs_streamwrapper/src/S3fsFileSystemService.php).

Full stack trace:

#0 /var/www/html/web/modules/contrib/s3fs/modules/s3fs_streamwrapper/src/S3fsFileSystemService.php(228): Drupal\s3fs_streamwrapper\S3fsFileSystemService->prepareDestination('/tmp/gd_MM4B8R', 'public://styles...', Object(Drupal\Core\File\FileExists))
#1 /var/www/html/web/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php(372): Drupal\s3fs_streamwrapper\S3fsFileSystemService->move('/tmp/gd_MM4B8R', 'public://styles...', Object(Drupal\Core\File\FileExists))
#2 /var/www/html/web/core/lib/Drupal/Core/Image/Image.php(125): Drupal\system\Plugin\ImageToolkit\GDToolkit->save('/tmp/gd_MM4B8R')
#3 /var/www/html/web/core/modules/image/src/Entity/ImageStyle.php(334): Drupal\Core\Image\Image->save('public://styles...')
#4 /var/www/html/web/modules/contrib/s3fs/modules/s3fs_streamwrapper/src/Controller/S3fsImageStyleDownloadController.php(230): Drupal\image\Entity\ImageStyle->createDerivative('public://2024-0...', 'public://styles...')
#5 [internal function]: Drupal\s3fs_streamwrapper\Controller\S3fsImageStyleDownloadController->deliver(Object(Symfony\Component\HttpFoundation\Request), 'public', Object(Drupal\image\Entity\ImageStyle))
#6 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#7 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(638): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#8 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#9 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#10 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(181): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#11 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#12 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#13 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#14 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#15 /var/www/html/web/core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#16 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#17 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#19 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#20 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\AjaxPageState->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(741): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#24 {main}

Steps to reproduce

  1. Upload a media image at /media/add/image
  2. Confirm the default thumbnail image style doesn't display
  3. Check the error in the logs at /admin/reports/dblog

Proposed resolution

In Service Class: Drupal\s3fs_streamwrapper\S3fsFileSystemService
https://git.drupalcode.org/project/s3fs/-/blob/4.0.x/modules/s3fs_stream...

Update function prepareDestination based on Core function getDestinationFilename signature:
https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/lib/Drupal/...

to accept :
@param \Drupal\Core\File\FileExists|int $fileExists

An int or a \Drupal\Core\File\FileExists object.

I haven't had the time to investigate further when and how this change occurred, but found this is probably related :
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21File%21Fi...

FileSystemInterface::EXISTS_RENAME

Deprecated
in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\File\FileExists::Rename instead.

 

This is somewhat major as it prevents the proper display of images generated with image styles.
 

🐛 Bug report
Status

Needs work

Version

4.0

Component

Code

Created by

🇫🇷France dydave

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

Merge Requests

Comments & Activities

  • Issue created by @dydave
  • Merge request !60Initial attempt to fix 'TypeError:... → (Open) created by dydave
  • Pipeline finished with Failed
    8 months ago
    Total: 265s
    #227952
  • Status changed to Needs review 8 months ago
  • 🇫🇷France dydave

    This is my initial stab at this:

    I've created an initial merge request so I could get a patch to apply on my project, but I definitely wanted to run this by you Conrad (@cmlara).

    I didn't have much time to fix the problem, investigate why/when/how this change occurred in related core libraries and understand what is really happening under the hood or the global implications/impact these changes could have on the module in general, so your review, advice and feedback would be super appreciated.

    So far, the patch seems to fix the problem for image styles with my setup: Private bucket all the way and everything is fed through Streamwrapper (public and private files).

    The error has disappeared and the images are displayed properly.

    I'm aware, the merge request would definitely need more work, in particular, updating comments, variable names, etc...
    But I wanted to know if these changes were going in the right direction first, before spending more time on this...

    I'm setting this in Needs review so you could perhaps let me know at a first glance if there is anything I've changed that could cause other potential issues.

    Based on your advice, feedback, suggestions and recommendations, I would definitely be very happy to help testing, or making any change to the merge request, if that could help you in any way.

    Otherwise, feel free to let me know if you have any questions or would need more information on the bug itself, the ticket in general, or the merge request, I would surely try answering as soon as possible.
    Thanks very much in advance!

  • Status changed to Needs work 8 months ago
  • 🇺🇸United States cmlara

    Overall looks like you have the right idea on implantation changes required.

    Add support for D10.3 Active has some of the equivalent changes from the 8.x-3.x branch (classes are in different locations but have the same code heritage).

    Glancing through your changes it looks like you may have caught a few areas in the code that I missed (phpdocs) when updating the 8.x-3.x branch as I don’t recall updating those. We probably actually do need old and new references for the time being in the docs as both are allowed and may be processed through.

    I’ll note that the S3fsFileSystemD103 is the new class for 10.3+. It is mostly the same as the pre10.3 class and probably very close to the MR with some added BC of mapping INT’a to ENUM’s that probably should exist in our service too so that way we are natively speaking in cores target typedef with no need to worry about core dropping INT’s in D12.

    As we are not in alpha/beta we can make changes to the file system service without a new class so that is fine as is.

    I’ve been considering next week after core releases 11.x we could (since we have not even released an alpha yet) bump our minimum requirements to 11.x and avoid some of the BC logic that 3.x needed to have related to

    Long term it helps us to have a 4.0.0 release with as little BC layers as possible. Once we release I tend to hold that we do not drop support for a version of core until we next increase our major (right now 8.x-3.x can work on D8.8-D10.3 and with a small set of commits sitting in the NR queue will also support D11 as context of what we are working with regarding commitment to BC)

    Setting as needs work as I belive it will need at least the enum/int BC mapping changes brought in (we should convert inside our service as eventually core will drop integers and we will need to be sure to always pass ENUM’s to core) and a decision on what to bump core minimum too will determine if any other layers are needed.

  • 🇫🇷France dydave

    Thanks a lot Conrad (@cmlara), once again for the prompt feedback and for taking the time outline the necessary changes and roadmap, it's super appreciated! 🙏

    This one is going to take me a bit more time, to review all the points, related issue and make another round of changes to the MR 😅

    At least I know my initial round isn't going to break my site and is a first step in the right direction 👍

    I'll be following-up on this issue soon!
    Thanks!

Production build 0.71.5 2024