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

Console/Kernel timezone() and scheduleTimezone() stopped working between 8.52.0 and 8.53.0 #38283

Closed
jason-klein opened this issue Aug 9, 2021 · 4 comments
Labels

Comments

@jason-klein
Copy link

  • Laravel Version: 8.53.0
  • PHP Version: 8.0.9
  • Database Driver & Version: N/A

Description:

I noticed the following change in behavior after Laravel 8.53.1 update was applied last week. I confirmed this bug is present in 8.53.1 and appears to have been introduced in Laravel Framework 8.53.0.

Console/Kernel timezone() and scheduleTimezone() stopped working between 8.52.0 and 8.53.0. Daily scheduled commands began running 5 hours earlier than expected.

Commands that would normally run at 04:00 UTC-5 (America/Chicago) were suddenly running at 23:00 UTC-5 or 04:00 UTC.

We have been using the following snippet from the Laravel 8.x scheduling docs to set a default Timezone of 'America/Chicago' for several scheduled commands that need to run at the same local time each day throughout the year.

/**
 * Get the timezone that should be used by default for scheduled events.
 *
 * @return \DateTimeZone|string|null
 */
protected function scheduleTimezone()
{
    return 'America/Chicago';
}

We also tried setting timezone('America/Chicago') on the individual commands without success. Example command shown below:

        $schedule->command('example:command')
            ->timezone('America/Chicago')
            ->dailyAt('04:00');

I confirmed that reverting back to 8.52.0 immediately restored working functionality for Console/Kernel timezone() and scheduleTimezone(). Commands would run at specified time in local time (e.g. America/Chicago) instead of server time (e.g. UTC).

Steps To Reproduce:

  1. Downgrade to Laravel 8.52.0
composer update laravel/framework:8.52.0
  1. Configure default Console/Kernel timezone using scheduleTimezone() as described above or create a command and specify timezone() as described above.
  2. Confirm scheduled job runs at local time (in our case, America/Chicago)
  3. Upgrade to Laravel 8.53.0
composer update laravel/framework:8.53.0
  1. Confirm scheduled job runs at server time (in our case, UTC time) instead of in local time specified by scheduleTimezone() or timezone()
@jason-klein
Copy link
Author

The issue was introduced by change to src/Illuminate/Console/Scheduling/Event.php in this 8.x commit 259b934 (Merge branch '6.x' into 8.x) which was originally introduced in this 6.x commit 176904e (Remove hardcoded Carbon reference from scheduler event).

The issue is happening because our Laravel app overrides the Date Factory to use CarbonImmutable instead of Carbon in our AppServiceProvider. The issue does NOT occur if we remove the following Date Factory override:

public function register()
{
    ...
    DateFactory::use(CarbonImmutable::class);
}

I realized our immutable Date Factory may be causing problems when attempting to instantiate a $date variable using the new Date::now() that replaced Carbon::now() in the commits I listed above. NOTE that the final $date value has a UTC timezone since the second command does not assign the result back to $date.

artisan tinker
>>> $date = Illuminate\Support\Facades\Date::now();
=> Carbon\CarbonImmutable @1628476858 {#5175
     date: 2021-08-09 02:40:58.250206 UTC (+00:00),
   }
>>> $date->setTimezone('America/Chicago')
=> Carbon\CarbonImmutable @1628476858 {#5170
     date: 2021-08-08 21:40:58.250206 America/Chicago (-05:00),
   }
>>> $date
=> Carbon\CarbonImmutable @1628476858 {#5175
     date: 2021-08-09 02:40:58.250206 UTC (+00:00),
   }

Proposed Fix

$date->setTimezone($this->timezone);

    $date = $date->setTimezone($this->timezone);

I'd be glad to test and submit a PR for this one-liner, if you'd like?

We almost exclusively use CarbonImmutable. We configured this custom Date Factory so that our Eloquent Models would return CarbonImmutable dates. We can now cast immutable dates thanks to recent commit 1369ecc (Immutable date and datetime casting). I am updating our Models to use the new "immutable_date" cast and removing our custom Date Factory to work around this issue.

@driesvints
Copy link
Member

@jason-klein thanks for the report. Does it fixes it for you if you use the Date facade in your register method instead of DateFactory like the OP does here: #38063 ?

Seems like he made the change to specifically get the behavior you were already using.

@jason-klein
Copy link
Author

@driesvints Glad to help. No change when attempting to use the Date facade in my register method instead of the DateFactory as seen in PR #38063. Example:

Date::use(CarbonImmutable::class);

While using the Date facade in my register method, I confirmed our scheduled commands are still firing at server time (UTC) when attempting to specify timezone (America/Chicago) in Console/Kernel using either the timezone() method (for per-command timezone) or the scheduleTimezone() method (for default schedule timezone).

PS - Thank you for linking to that PR! Adding a test similar to the one shown in the PR to our app so we can make sure these commands that are intended to run at local time are, indeed, running at local time instead of server time.

@driesvints
Copy link
Member

This'll be in tomorrow's release. Sorry about this and thanks for reporting!

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

No branches or pull requests

2 participants