Skip to content

Fix impossible function cron schedules#12416

Merged
ChiragAgg5k merged 3 commits into
1.9.xfrom
fix/schedule-functions-impossible-cron
May 29, 2026
Merged

Fix impossible function cron schedules#12416
ChiragAgg5k merged 3 commits into
1.9.xfrom
fix/schedule-functions-impossible-cron

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

What does this PR do?

Prevents impossible function cron expressions from crashlooping the function scheduler.

The cron library can parse expressions like */5 22-3 * * *, but later throws RuntimeException: Impossible CRON expression when calculating the next run date. This PR validates cron expressions by calculating the next run date and makes the function scheduler silently skip impossible legacy schedules.

Test Plan

  • vendor/bin/phpunit tests/unit/Task/Validator/CronTest.php --display-warnings
  • composer lint src/Appwrite/Task/Validator/Cron.php
  • composer lint src/Appwrite/Platform/Tasks/ScheduleFunctions.php
  • composer lint tests/unit/Task/Validator/CronTest.php

Note: The targeted PHPUnit run passes assertions but reports a warning from dragonmantank/cron-expression while evaluating the intentionally invalid wrapped-hour expression.

Related PRs and Issues

  • #XXXX

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR fixes a crashloop in the function scheduler caused by cron expressions that are syntactically valid but impossible to execute (e.g., */5 22-3 * * * with a wrapped-hour range). The fix applies in two places: the Cron validator now rejects such expressions at save time, and the scheduler loop catches RuntimeException to silently skip any impossible legacy schedules already stored in the database.

  • Cron.php: Wraps getNextRunDate() in a try/catch so impossible expressions are rejected during input validation, preventing them from ever being persisted.
  • ScheduleFunctions.php: Moves getNextRunDate() inside the existing try block and adds a RuntimeException catch branch, so pre-existing impossible schedules in the database no longer crash the enqueue loop.
  • CronTest.php: Adds two assertions — one for the valid comma-separated cross-midnight form (22-23,0-3) and one for the invalid wrapped-range form (22-3) — directly exercising the new validation path.

Confidence Score: 5/5

Safe to merge — the change is minimal and targeted, touching only the validation path and the scheduler's exception-handling block.

Both code sites are straightforward: the validator adds a single try/catch around an existing library call, and the scheduler moves one line inside an already-guarded block. The new test cases directly cover the happy path and the failure path introduced by this change. No existing behavior is removed or altered beyond expanding the set of expressions that are treated as invalid.

No files require special attention.

Important Files Changed

Filename Overview
src/Appwrite/Task/Validator/Cron.php Adds getNextRunDate() execution check inside a try/catch to reject syntactically-valid but impossible cron expressions at validation time.
src/Appwrite/Platform/Tasks/ScheduleFunctions.php Moves getNextRunDate() inside the try block and adds a RuntimeException catch so impossible legacy schedules are silently skipped instead of crashing the scheduler loop.
tests/unit/Task/Validator/CronTest.php Adds test cases for the valid comma-separated cross-midnight range (/5 22-23,0-3) and the invalid wrapped-range expression (/5 22-3) to exercise the new validation logic.

Reviews (3): Last reviewed commit: "Revert "Fix deployment cancel race in bu..." | Re-trigger Greptile

Comment thread tests/unit/Task/Validator/CronTest.php
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 28, 2026

✨ Benchmark results

Comparing 1.9.x (before) to fix/schedule-functions-impossible-cron (after).

Before

Scenario P50 (ms) P95 (ms) Requests RPS
API total 15.24 145.94 185 32.51
Account 26.94 173.1 35 6.72
TablesDB 15.2 23.01 35 7.99
Storage 12.8 48.84 75 16.87
Functions 21.28 31.09 40 9.19

After

Scenario P50 (ms) P95 (ms) Requests RPS
API total 17.25 149.36 185 30.78
Account 28.52 200.18 35 6.35
TablesDB 15.84 24.57 35 7.46
Storage 14.37 50.79 75 15.69
Functions 22.5 36.08 40 8.48

Delta

Scenario P95 delta (ms)
API total +3.42
Account +27.08
TablesDB +1.57
Storage +1.95
Functions +4.99
Top API waits
API request Max wait (ms)
account.sessions.email.create 642.6
account.password.update 200.03
account.create 161.88

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 28, 2026

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 9b45853 - 1 flaky test
Test Retries Total Time Details
RealtimeCustomClientTest::testChannelDatabase 1 40ms Logs
Commit 902b1d5 - 1 flaky test
Test Retries Total Time Details
DocumentsDBCustomServerTest::testTimeout 1 126.40s Logs
Commit 939d2f4 - 1 flaky test
Test Retries Total Time Details
LegacyCustomServerTest::testTimeout 1 131.40s Logs

@ChiragAgg5k ChiragAgg5k merged commit 4b059dd into 1.9.x May 29, 2026
83 of 85 checks passed
@ChiragAgg5k ChiragAgg5k deleted the fix/schedule-functions-impossible-cron branch May 29, 2026 08:35
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

Successfully merging this pull request may close these issues.

2 participants