PoStreamReader::readLine() throws an error on module install

Patch in #7 works for me on Drupal 9.5.2!

As a bug this will need a test case to show the error.

Patch #7 resolved bug for 9.5.7 core version.
thank you @iperiba92

I had the same problem. I updated translations again here /admin/reports/translations and the problem is gone

Thank you for patch #7!

I started getting this error recently, preventing module installation. The patch allowed me to install again as normal without fatal errors.

In our case it happened on core 9.5.10. The temporary folder was not configured correctly. After adding

$settings['file_temp_path'] = 'tmp';

the problem went away.
So, it's worth checking if your tmp folder is correctly configured and if it has the correct permissions before applying a patch.

I just ran into this in a local ddev enviroment and #16 worked. Perhaps it's a file / folder permission issue?

Also see https://vazcell.com/blog/error-activating-module-drupal-9-translations-a...

For me the solution was:
- visit /admin/config/media/file-system
- make sure a translations folder is set up there, like: 'sites/default/files/translations'.
- Save the page

Now it is fixed, if I enable another module, or drush pmu and then drush en my module again, all works good.
I should note that when I visited /admin/config/media/file-system, a translations folder was already set up there.
But the actual folder that is set up there was missing on my local dev environment.
By saving that page without making any changes, Drupal created the translations folder for me.

I faced this issue after upgrading from Drupal 9.5.11 to Drupal 10.1.5.
The translations directory exists and the permissions are fine.
The temporary directory is configured correctly.
It works only after applying the patch!

I faced this issue with drupal 9.3.22 when we upgraded from PHP 7.3 to php 8.1. The comment #21 worked for me. I just saved the page without making any changes and the error was gone. When I do "drush locale-update" no more error are shown.

Upgrading from 9.5.11 to 10.1.7, I had the same issue. What fixed it for me was a mix of #21 🐛 PoStreamReader::readLine() throws an error on module install Needs work and other things:

• visiting /admin/config/media/file-system
• making sure a translations folder is set up there, like: 'sites/default/files/translations'.
• saving the form
• running drush locale:check then drush locale:umpdate
• finally, in /admin/reports/status, clicking on the run cron button, I get an error message saying that it failed. I clicked again and now it's working

Hope that helps. No patch was needed for me.

Patch on #7 works for me on Drupal 10.1.7. In my case, the error showed up while performing a "drush deploy" in local DDEV environment. So I think it's important to show the error message but avoid interrupting the configuration import process only due to translations import failures.

Patch in #7 works for me on Drupal 10.2.3.

The patch works, but I don't think it's the correct approach here, we're simply ignoring an error and/or misconfiguration issue.
In my opinion this should be fixed by configuring correctly the file-system options and directory permissions.

Agree with #28 -- this is not the right fix.

The right fix would be for the problem to produce a warning that tells users that they need to configure the temp folder.

Yeah this is not a bug, but a configuration error on the user's behalf. I hit this on a CI environment because the translations folder was missing.

I guess we can improve the error message.

#30 is right and the way to go. Exactly. Still I think for the results it might be seen as (at least ux) bug (WSOD)

Most important is, that this doesn't cause a WSOD and non-technical users are entirely blocked...

In my case this error blocks the deploy pipeline when installing new modules. I agree it's a server miss configuration, but it's not enough reason to block the deployment. So, apart from improving the error message, we should keep changes from current patch.

Came across this issue and used the patch. Not because translation directory wasn't configured properly but just because it couldn't download with a drush locale:update:

ddev drush locale:update
>  [error]  Unable to download translation file https://ftp.drupal.org/files/translations/all/admin_toolbar/admin_toolbar-3.5.1.nl.po.
>  [warning] filesize(): stat failed for //admin_toolbar-3.5.1.nl.po locale.bulk.inc:203
>  [warning] fopen(translations://admin_toolbar-3.5.1.nl.po): Failed to open stream: "Drupal\locale\StreamWrapper\TranslationsStream::stream_open" call failed PoStreamReader.php:154
>  [error]  TypeError: fgets(): Argument #1 ($stream) must be of type resource, bool given in fgets() (line 248 of /var/www/html/web/core/lib/Drupal/Component/Gettext/PoStreamReader.php) #0 /var/www/html/web/core/lib/Drupal/Component/Gettext/PoStreamReader.php(248): fgets(false)...

Patch saved my day. Rather have a broken translation than trying to figure out why it's unable to download translation. Agree with #34.

guiu.rocafort.ferrer → made their first commit to this issue’s fork.

I created a new branch from the targeted Drupal version 11.x in the issue and applied the previously commited change in the old branch by @immaculate.x. I reproduced the issue with the change, and now the command "drush pm:enable paragraphs" succeeds, but it gives the following errors:

> [notice] Checked es translation for paragraphs.
> [notice] Checked it translation for paragraphs.
> [notice] Checked de translation for paragraphs.
> [error] Unable to download translation file https://ftp.drupal.org/files/translations/all/paragraphs/paragraphs-8.x-....
> [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/paragraphs-8.x-1.18.es.po locale.bulk.inc:204
> [warning] fopen(translations://paragraphs-8.x-1.18.es.po): Failed to open stream: "Drupal\locale\StreamWrapper\TranslationsStream::stream_open" call failed PoStreamReader.php:154
> [notice] Unable to import translations file: translations://paragraphs-8.x-1.18.es.po
> [error] Unable to download translation file https://ftp.drupal.org/files/translations/all/paragraphs/paragraphs-8.x-....
> [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/paragraphs-8.x-1.18.it.po locale.bulk.inc:204
> [warning] fopen(translations://paragraphs-8.x-1.18.it.po): Failed to open stream: "Drupal\locale\StreamWrapper\TranslationsStream::stream_open" call failed PoStreamReader.php:154
> [notice] Unable to import translations file: translations://paragraphs-8.x-1.18.it.po
> [error] Unable to download translation file https://ftp.drupal.org/files/translations/all/paragraphs/paragraphs-8.x-....
> [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/paragraphs-8.x-1.18.de.po locale.bulk.inc:204
> [warning] fopen(translations://paragraphs-8.x-1.18.de.po): Failed to open stream: "Drupal\locale\StreamWrapper\TranslationsStream::stream_open" call failed PoStreamReader.php:154
> [notice] Unable to import translations file: translations://paragraphs-8.x-1.18.de.po
> [notice] The configuration was successfully updated. 370 configuration objects updated.
> [error] Message: Failed to save file due to error "/The specified file
> 'temporary://fileSRCiNi' could not be copied because the destination
> directory 'translations://' is not properly configured. This may be caused by
> a problem with file or directory permissions./"
>
> [error] Message: Failed to save file due to error "/The specified file
> 'temporary://filePFV9Fz' could not be copied because the destination
> directory 'translations://' is not properly configured. This may be caused by
> a problem with file or directory permissions./"
>
> [error] Message: Failed to save file due to error "/The specified file
> 'temporary://fileWiy0wK' could not be copied because the destination
> directory 'translations://' is not properly configured. This may be caused by
> a problem with file or directory permissions./"
>
> [error] Message: 6 translation files could not be imported. See the log for details.
>
> [notice] Message: The configuration was successfully updated. There are /370/ configuration
> objects updated.

Notice that the errors already says that the directory translations might be not properly configured, so i think this should be good enough to not generate a WSOD, and also display the appropiate error messages to the user about the translation folder not being correctly configured.

Thanks for continuing this forward. Was previously tagged for test coverage which still appear to be needed, so NW for that.

pfrenssen → changed the visibility of the branch 3301239-postreamreaderreadline-throws-an to hidden.

pfrenssen → changed the visibility of the branch 11.x to hidden.

I've hidden the outdated branches, the branch from @guiu.rocafort.ferrer is the current focus :) Thanks a lot working on this!

Hi @pfrenssen, thanks for your comments.

I think a good approach would be to throw an exception in open when fopen fails to open the stream.

Regarding readItem, i would leave the readLine fix still there, in case someone calls the method accidentally. The method might already return NULL if the file has some errors in it or if it is empty, so i think there is no warm in leaving it for now.

I will add the exception to the open method, and also leave the already implemented fix in readLine, and add a new test to validate the open exception.

When this issue is closed, we can create a separate issue to review the whole PoStreamReader class and add more exhaustive unit testing to all the methods.

One of the tests is failing because php is throwing a warning message, and it makes phpunit treat fail the test...

Thanks @pfrenssen for fixing the failing test.

I've tried again enabling a module with the translations folder having the wrong permissions and it outputs the following messages:

>  [notice] Checked de translation for webform.
>  [notice] Checked es translation for webform.
>  [notice] Checked ca translation for webform.
>  [error]  Unable to download translation file https://ftp.drupal.org/files/translations/all/webform/webform-6.3.0-beta1.de.po. 
>  [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/webform-6.3.0-beta1.de.po locale.bulk.inc:204
>  [notice] Unable to import translations file: translations://webform-6.3.0-beta1.de.po
>  [error]  Unable to download translation file https://ftp.drupal.org/files/translations/all/webform/webform-6.3.0-beta1.es.po. 
>  [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/webform-6.3.0-beta1.es.po locale.bulk.inc:204
>  [notice] Unable to import translations file: translations://webform-6.3.0-beta1.es.po
>  [error]  Unable to download translation file https://ftp.drupal.org/files/translations/all/webform/webform-6.3.0-beta1.ca.po. 
>  [warning] filesize(): stat failed for /var/www/html/web/sites/default/files/translations/webform-6.3.0-beta1.ca.po locale.bulk.inc:204
>  [notice] Unable to import translations file: translations://webform-6.3.0-beta1.ca.po
>  [notice] The configuration was successfully updated. 398 configuration objects updated.
>  [error]  Message: Failed to save file due to error "/The specified file  
> 'temporary://fileB2wo3R' could not be copied because the destination  
> directory 'translations://' is not properly configured. This may be caused by  
> a problem with file or directory permissions./"
>  
>  [error]  Message: Failed to save file due to error "/The specified file  
> 'temporary://fileSQbYRB' could not be copied because the destination  
> directory 'translations://' is not properly configured. This may be caused by  
> a problem with file or directory permissions./"
>  
>  [error]  Message: Failed to save file due to error "/The specified file  
> 'temporary://fileUBLW8Q' could not be copied because the destination  
> directory 'translations://' is not properly configured. This may be caused by  
> a problem with file or directory permissions./"
>  
>  [error]  Message: 6 translation files could not be imported. See the log for details.
>  
>  [notice] Message: The configuration was successfully updated. There are /398/ configuration  
> objects updated.
> 

The command does not fail, but it also displays enough error messages to point up to the user there is a problem with the translations folder permissions, there are also test for validating the changes made, so i am marking it as needs review.

Thanks for the updates @VladimirAus, I am really happy with how this is shaping up. Not only is the PHP error solved in a clean way, but we also have a helpful exception that can be caught by calling code and gives useful information about the underlying problem.

I would like to remove the second test because I think it is unnecessary and would make future maintenance of the class harder. And we have a PHPStan warning. I left comments on the MR.

GREAT work @vladimiraus and @pfrenssen!! Thank you! Happy to see this landing soon :)

@vladimiraus, i appreciate the refactor and improving the code, it looks way better this way, but please, next time try not to introduce validation errors, it was passing all the tests before the change. I fixed it already.

@pfrenssen I guess that now that the open method returns an exception when it cannot open the file, it is way less likely that readItem is called, but it is still not impossible. I think it would be a good idea to allow readItem to throw an exception when the fd is not valid, but i am worried that this might break existing code which is not expecting the method to be thrown.

When I try to run cron or update the translations, I receive the error below:

TypeError: fgets(): Argument #1 ($stream) must be of type resource, false given in fgets() (line 248 of /var/www/html/web/core/lib/Drupal/Component/Gettext/PoStreamReader.php).

I guess, it is related to this error.

The patch file based on MR has been provided.