Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Issue with the new maintenance mode #34085

Closed
arcanedev-maroc opened this issue Sep 1, 2020 · 17 comments
Closed

[8.x] Issue with the new maintenance mode #34085

arcanedev-maroc opened this issue Sep 1, 2020 · 17 comments

Comments

@arcanedev-maroc
Copy link
Contributor

arcanedev-maroc commented Sep 1, 2020

  • Laravel Version: 8.0-dev
  • PHP Version: 7.4.9
  • Database Driver & Version: none
  • OS: Windows 8.1

Description:

I can't use the new maintenance mode on my machine because it's caused by this line:

<?php
// storage/framework/maintenance.php

// Check if the application is in maintenance mode...
if (! file_exists($down = __DIR__.'/../storage/framework/down')) { // <-------------- this one
    return;
}

// The rest of the file...

The file_exists($down = __DIR__.'/../storage/framework/down') always returns false.

Steps To Reproduce:

  • Disable/comment the \App\Http\Middleware\PreventRequestsDuringMaintenance middleware
  • Run php artisan down --render="errors::503"
  • Run php artisan serve
  • Visit the website

Solution

I fixed the issue by replacing the path __DIR__.'/../storage/framework/down' by __DIR__.'/down'

@arcanedev-maroc
Copy link
Contributor Author

cc @driesvints

@taylorotwell
Copy link
Member

Eh, have you moved around your directories away from the default structure? Works fine for me using a fresh Laravel app.

@arcanedev-maroc
Copy link
Contributor Author

No, i've tested with a fresh laravel (develop) installation

@taylorotwell
Copy link
Member

Works fine for me. The file is DEFINITELY not in DIR.'/down'.

@willrowe
Copy link
Contributor

willrowe commented Sep 2, 2020

I think the issue here is that it is difficult to test how this is not working as expected. All of the functionality of maintenance mode is technically working except the use case of the maintenance.php file bypassing booting of the framework. In order for this to work, the exit calls must be reached when either a redirect or template is specified.

The reason it appears to be working is that all of the redirect, secret, and template functionality is also implemented in the PreventRequestsDuringMaintenance middleware. So when file_exists($down = __DIR__.'/../storage/framework/down') fails and returns back to the index.php file, eventually that middleware will be hit making it appear that everything is functioning correctly.

I believe the best way to test this is to disable the PreventRequestsDuringMaintenance middleware, maintenance mode should still work if a template is provided. If you create a 503.blade.php template and then call php artisan down --render=503, remove the PreventRequestsDuringMaintenance middleware from the HTTP kernel and then visit the site, it should show whatever was entered into the 503.blade.php. However, this is not the case, as it is still showing the welcome.blade.php template.

As @arcanedev-maroc mentioned, the issue is that the maintenance.php stub is copied into the storage/framework directory, which is the same directory where the down file is created. This means that the path passed to file_exists will never exists because it is actually checking for storage/storage/framework/down instead of storage/framework/down.

@arcanedev-maroc
Copy link
Contributor Author

Hi @driesvints @taylorotwell

As @willrowe mentioned above, the best way to test this issue is to disable \App\Http\Middleware\PreventRequestsDuringMaintenance middleware to see if the storage/framework/maintenance.php file is working or not, after running the php artisan down --render="errors::503" command.

@willrowe
Copy link
Contributor

willrowe commented Sep 8, 2020

To be completely clear, I believe this is indeed a bug and this issue should be re-opened. Seeing as this feature is supposed to be released today as part of v8, I am concerned that this could cause some issues when used in production that will not be caught until it is too late. I could be doing something wrong, but I think it is at least worth following the steps outlined above to test if the new maintenance mode features are indeed functioning as expected.

@Hesammousavi
Copy link
Contributor

i have this issues too.

Laravel Version: 8.0
OS: Windows 10

@arcanedev-maroc
Copy link
Contributor Author

ping @driesvints

@driesvints
Copy link
Member

@arcanedev-maroc I'll try to reproduce this tomorrow.

@snapey
Copy link

snapey commented Sep 10, 2020

A simple way to prove this is by adding a simple syntax error to the framework code, for instance, in web.php

Maintenance.php should prevent the framework from booting as a result of a web request, but it does not. Whilst the application has any error then the end user sees a 500 error and not the intended down message.

Issue should be renamed to reflect that this is only for the Pre-rendered maintenance mode view

@driesvints
Copy link
Member

Checking into this again and trying to wrap my head around this. Couple of things:

  1. Give me a way to reproduce this without disabling the PreventRequestsDuringMaintenance middleware as you shouldn't be doing that.
  2. You mentioned: it is actually checking for storage/storage/framework/down instead of storage/framework/down which I don't believe to be true. It detects my down file and I get my 503 maintenance screen

If you can give me steps to reproduce this without disabling parts that the maintenance functionality obviously needs then I can look into this again.

@willrowe
Copy link
Contributor

From my comment here:

Perhaps the simplest way to see whether this is working correctly or not would be to add:

var_dump('bypassing framework');
die

after this block and:

var_dump('using framework');
die

after this block. Then you can run the down command using different options and see whether it is bypassing the framework when you expect it to.


The difficulty here is that it will not be reproducible except when there is an issue with the framework not being available during a composer update or something like that, which is why the new options are supposed to bypass booting of the framework. If you are testing with the framework fully intact then it will work without any issue, so you need to test it for the case these options were made for. That is why I suggested removing the middleware, to simulate something in the framework missing. When you are bypassing the framework, it should not matter that the middleware is missing, since it should never be reached.

@willrowe
Copy link
Contributor

  1. You mentioned: it is actually checking for storage/storage/framework/down instead of storage/framework/down which I don't believe to be true. It detects my down file and I get my 503 maintenance screen

A simple way to check this would be to var_dump out the path it is checking in the file_exists call.

@arcanedev-maroc
Copy link
Contributor Author

arcanedev-maroc commented Sep 11, 2020

@driesvints, if you read the release note about the new maintenance mode (Pre-Rendering The Maintenance Mode View)

If you utilize the php artisan down command during deployment, your users may still occasionally encounter errors if they access the application while your Composer dependencies or other infrastructure components are updating. This occurs because a significant part of the Laravel framework must boot in order to determine your application is in maintenance mode and render the maintenance mode view using the templating engine.

For this reason, Laravel now allows you to pre-render a maintenance mode view that will be returned at the very beginning of the request cycle. This view is rendered before any of your application's dependencies have loaded. You may pre-render a template of your choice using the down command's render option:

php artisan down --render="errors::503"

Run php artisan down --render="errors::503" command and make an error or abort() in routes/web.php (for example). You should see the 503 as an expected behavior. But it's not the case.

<?php

use Illuminate\Support\Facades\Route;

/*
|--------------------------------------------------------------------------
| Web Routes
|--------------------------------------------------------------------------
|
*/

Route::get('/', function () {
    return view('welcome');
});

abort(404);

@driesvints
Copy link
Member

We've finally managed to reproduce the path error and are looking into fixing this. Thanks for reporting and your patience 👍

@driesvints
Copy link
Member

We merged #34264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants