Catch error(s) from composer install not being run and return friendly error

Spitballing here.

Another spitball that doesn't add a variable to the global scope.

Not sure if this is the right way to approach it, but the error message (wherever it is) should include a link to the composer documentation for those who go "Composer? What does Mozart have to do with this?"

Agreed... Or a link to a Drupal.org doc page on the setup, so we could make it more Drupal-specific.

1. +++ b/autoload.php
   @@ -11,4 +11,11 @@
   +if (!file_exists(__DIR__ . '/vendor/autoload.php')) {
   ...
   +else {
   +  return require __DIR__ . '/vendor/autoload.php';
   +}
   

   Could we not do a include_once and check the return value

2. +++ b/autoload.php
   @@ -11,4 +11,11 @@
   +  header('500 Internal Server Error');
   +  print 'Run composer install in your web root.';
   +  exit;
   

   Ideally we should point onto drupal.org explaining people to either run composer install or use the packages provided by Drupal.

Could we not do a include_once and check the return value

autoload.php is the source of the autoloader object. Its return value is important.

Also, we want it to explode if it's present but malformed in some way.

The only way to do this is to do a file_exists() on every page load. However, the autoloader can be moved, and in fact *must* be moved for sane deployment as with https://github.com/drupal-composer/drupal-project , so the file_exists() will be checking the wrong place in many circumstances.

Big -1 on this issue. The user will see that there is a problem, will find out that we have a lot of documentation on the issue, and then will learn how to use Composer.

We should instead provide a how-to-install text file at the root level of the repo, which currently does not exist.

My initial reaction is to agree with Mile23, seems like something that could be handled with just documentation. You're going to have to follow an installation guide anyway for some of the other requirements.

#8 makes sense but errors should be user friendly

I agree with #10, this issue is all about providing a helpful error message.

Patch #4 looks a lot better than the file_exists(), for me I'd just add an error message to that and remove the @ error suppression - it's fine to show the PHP warning in this situation.

If we don't like the $autoloader return check, we could also do a class_exists() on DrupalKernel which is the next thing that fails.

I'm trying something slightly different. Based on @catch's suggestion in #12, I am checking for the class itself. I am just changing the require to include in autoload.php.

I agree with @Mile23's concerns but I am not sure how they apply here. We are modifying the autoload.php which is supposed to be edited by the user if they place the vendor directory elsewhere. The drupal-composer project (with drupal-scaffold) edits this file with the actual relative vendor path. Now, if the file has been edited in such a way, it means either that the user knows about composer and would recognize the error or the user has already run composer install, in which case we don't need to worry anymore.

On the other hand, if the user doesn't know about composer and sees the error, they would just have to follow the documentation. It is unlikely that they would be affected by moving the vendor elsewhere. If they want to move it, they have to again edit the autoload.php which is already a requirement.

Please let me know if I am missing something else.

In case it's not clear. I am changing require to include to avoid a fatal when the file doesn't exist. This approach also handles problems when the file is present but not valid in some way (as raised in #8). I am also aware that there might be a problem which might allow DrupalKernel to load but not other classes, but that seems like an extreme edge case and not the problem described in the issue (which is that the user has forgotten to run composer install).

+++ b/index.php
@@ -13,6 +13,12 @@
+  print("We can't find DrupalKernel class which is usually an indication that composer install hasn't been run. Refer to <a href='https://www.drupal.org/documentation/install/download#git'>this page</a> for more details.");

Yeah I would argue that we should not mention that DrupalKernel wasn't found ...

If you look at drupal/core composer.json autoload section you'll see that \Drupal is the most performant class lookup.

Also +1 on saying what the error is rather than what we looked up.

Please find attached a patch with the suggested changes. I have revised the error message to

It does not look like <code>composer install</code> has been run. Refer to <a href='https://www.drupal.org/documentation/install/download#git'>the drupal installation guide on drupal.org</a> for more details.

I have also changed the class being tested to \Drupal

I have prepared this against 8.2.x. Hope that is the right way to do this.

1. Checking the return object of include is a bit cleaner (it's the Composer loader object)
2. Ignoring the warning from include will save a PHP fatal error from happening

+++ b/autoload.php
@@ -11,4 +11,14 @@
+$autoloader = @include __DIR__ . '/vendor/autoload.php';

If not having installed with Composer were the only possible error with this, then @include might be OK, but it's not.

If we were normalizing on PHP 7, we could do this easily with a try/catch on \Error, but we can't.

That leaves poking around in error_get_last() and figuring out what to tell the user.

+++ b/autoload.php
@@ -11,4 +11,14 @@
+  print 'You must set up the project dependencies using <code>composer install

.' . PHP_EOL .
+ 'Refer to the the Drupal installationg guide for instructions on installing with Composer.';

This not clear that problem in autoloader

I ran into the same problem, and the patch in #19 seems like a clean way to solve this.

Mile23: what other causes except for Composer could this have?

RE: $autoloader = @include __DIR__ . '/vendor/autoload.php';

Mile23: what other causes except for Composer could this have?

If there's no vendor/autoload.php file, or a symlink can't be reached, or apache doesn't have read access to it, or whatever else, then I want to learn that from the error message.

So swallowing the error with @include is bad. We should test what it does, not whether it's there.

Much prefer #18, which looks for \Drupal and then complains if it doesn't work.

Agreed. So let's mark this for review again?

#18 helped me solve this issue when I ran into it earlier today.

Patch #18 does not work with Drush.

Patch #19 still needs work as pointed by @Mile23 at #24

@Mac_Weber, updating how Drush throws this error seems out of scope of this bug. I *think* the goal here is to provide a meaningful error to a site-builder/developer who installs the site, but hasn't run `composer install`. Currently when I run a drush command on a site without running `composer install`, I get the following error:

include(~/Sites/drup8/vendor/autoload.php): failed to open stream: No such [warning]
file or directory autoload.php:14
include(): Failed opening '~/Sites/drup8/vendor/autoload.php' for inclusion[warning]
(include_path='.:') autoload.php:14
PHP Fatal error:  Class 'Drupal\Core\Session\AccountInterface' not found in ~/Sites/drup8/core/includes/bootstrap.inc on line 60

Fatal error: Class 'Drupal\Core\Session\AccountInterface' not found in ~/Sites/drup8/core/includes/bootstrap.inc on line 60
Drush command terminated abnormally due to an unrecoverable error.                      [error]
Error: Class 'Drupal\Core\Session\AccountInterface' not found in
~/Sites/drup8/core/includes/bootstrap.inc, line 60

In other words, I would argue it's up to Drush to provide a more meaningful error, if it's required. Therefore, patch #18 from anoopjohn is sufficient.

@icicleking, if the intention is to give meaningful information to developers then displaying that message on Drush makes sense. IMHO, most developers do not install using the GUI.

I'm adding a patch that merges both solutions and I changed the error message to something that gives the necessary hints to developers for how to solve the issue with the vendor folder and the dependency files.

  print 'Dependencies error! The system cannot access some files at the vendor folder.<br>' . PHP_EOL .
        'Did you run composer install? You may also need to check the folder permissions, too.<br>' . PHP_EOL .
        'Refer to the <a href="https://www.drupal.org/documentation/install/download#git">the Drupal installationg guide</a> for instructions on installing with Composer.' . PHP_EOL;

In addition, I did many tests. One of them was deleting the file /vendor/autoload.php, but keeping all other files. In this test the installation worked on the GUI with patch #18. Then, I think this is also one more reason for merging both patches.

I added '<br>' . PHP_EOL because one works on browsers and the other only works at command line, giving a better output ;)

Tested it thoroughly and works as expected. One remark: we should probably use <br/> instead of <br>.

Sorry, wrong patch. Here's the right one.

Note that issues like this should be targeted against 8.4.x, but can be considered for backport once they are committed to that branch. See the updated alpha release policy. Thanks!

Patch against 8.4.x-dev.

Some minor English fixes to the patch in #35:

  print 'Dependencies error! The system cannot access some files in the vendor folder.' . PHP_EOL .
        'Did you run composer install? You may also need to check the folder permissions.' . PHP_EOL .
        'Refer to the the Drupal installation guide for instructions on installing with Composer.' . PHP_EOL;

Revisions
at the vendor folder -> in the vendor folder
", too" in the second line is redundant (because of also), so I removed it.
installationg -> installation

Changes made as per comment #36.

Setting to RTBC. The error message is suffiently clear.

1. +++ b/autoload.php
   @@ -11,4 +11,14 @@
   +if (!$autoloader) {
   
   +++ b/index.php
   @@ -13,6 +13,14 @@
   +if (!class_exists('\Drupal')) {
   +  header('500 Internal Server Error');
   +  print 'Dependencies error! The system cannot access some files in the vendor folder.<br/>' . PHP_EOL .
   +    'Did you run composer install? You may also need to check the folder permissions.<br/>' . PHP_EOL .
   +    'Refer to the <a href="https://www.drupal.org/documentation/install/download#git">the Drupal installation guide</a> for instructions on installing with Composer.' . PHP_EOL;
   +  exit;
   

   Why not change this to if (!$autoloader || !class_exists('\Drupal)) { and remove the changes from index.php.

2. +++ b/autoload.php
   @@ -11,4 +11,14 @@
   +  print 'Dependencies error! The system cannot access some files in the vendorfolder.<br/>' . PHP_EOL .
   +        'Did you run composer install? You may also need to check the folder permissions.<br/>' . PHP_EOL .
   +        'Refer to the <a href="https://www.drupal.org/documentation/install/download#git">the Drupal installation guide</a> for instructions on installing with Composer.' . PHP_EOL;
   +  exit();
   

   I'm not sure about the text Dependencies error! - I think the exclamation mark is unnecessary. Perhaps the entire test might be better it is more to the point. Something like: Failed to set up the autoloader. This can occur because composer install has not been run. Refer to https://www.drupal.org/docs/8/install/step-2-install-dependencies-with-composer for more information.

   I don't think the message should contain HTML or even set headers as this can cause problems when this occurs from the the CLI. However I think we should do exit(1);.

   Finally the link in the message in the patch should be https://www.drupal.org/docs/8/install/step-2-install-dependencies-with-c... whatever text we go for. It's good to see we're not really recommending starting from a git checkout on the installation instructions anymore.

How about this? Tested manually with both apache and drush, and seems to work fine.

LGTM.

The link has to be https://www.drupal.org/docs/installing-drupal/step-2-install-dependencie... .

Attached reroll patch against 9.5.x
Addressed the comment #53 and the link has been changed.

Great, thanks!

Thanks for working on this issue. Helpful errors are helpful!

Given

• the minimum version of Drupal is now 7.3
• The only recommended way to install drush is via composer

I think we can revisiting the solution.

The point of the issue is to emit a helpful message when composer has not run. I don't think that suppressing the warning about the file not existing is necessary. So we can now do:

try {
  return require __DIR__ . '/vendor/autoload.php';
}
catch (\Error $e) {
  if (PHP_SAPI !== 'cli') {
    header('500 Internal Server Error');
  }
  print 'Failed to set up the autoloader. This can occur because composer install has not been run. Refer to https://www.drupal.org/docs/installing-drupal/step-2-install-dependencies-with-composer for more information.' . PHP_EOL;
  exit(1);
}

Also one thing that is fun is that this is way less of a problem now that the recommended way to start a project is using composer as detailed on https://www.drupal.org/docs/develop/using-composer/starting-a-site-using...

So the only people who face this issue are people working on core development using the git checkout. This realisation indicates some things:

• The message is not really that helpful - this only affects people working on core but we are taking them to a page about installing a site.
• Are we really sure that we should do this? This means the root autoloader.php will be different from the one on 99% sites because that file is generated via \Drupal\Composer\Plugin\Scaffold\GenerateAutoloadReferenceFile::autoLoadContents()
• Note that there is no point \Drupal\Composer\Plugin\Scaffold\GenerateAutoloadReferenceFile::autoLoadContents() having this as the only way to get the autoload.php is because you've run composer

I think given the existence of the composer templates and the fact the tarball includes everything you need we should close this issue as outdated. Note that when this issue was open the composer templates did not exist.

Discussed with @catch and we both agree that this has become outdated due to the existence of the composer project templates and the fact that they are the recommended way of installing Drupal. Unless there is a valid point to do not close this issue I will mark it outdated on the 4th July.

Reroll the patch #39 with Drupal 10.0.x

As mentioned in #58, this is now outdated.