Skip to content

Conversation

@ingoratsdorf
Copy link
Contributor

@ingoratsdorf ingoratsdorf commented Sep 27, 2025

Ref: Issue #1199

If the schedule input is incorrect, an error message is logged and the plugin will NOT run. Creating a dummy schedule would throw the system out of balance as there's the danger of schedules running out of sync.

Error message in log:
16:57:20 ["[Config] [ERROR] Invalid schedule '*/2 * * * 5* *' for plugin 'INTRNT'. Error: Invalid cron string format."]

The settings screen will also show a warning:
image

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability for scheduled runs by validating cron expressions before applying them.
    • Prevents invalid or empty schedules from being added, reducing runtime errors.
    • Provides clearer error messages when schedule configuration is invalid.
    • Adds safeguards to log unexpected issues during schedule setup, improving diagnosability.

is the schedule input is incorrect, an error message is logged and the plugin will NOT run.
Creating a dummy schedule would throw the system out of balance as there's the danger of schedules running out of sync.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 27, 2025

Walkthrough

Adds guarded initialization of plugin schedules when RUN == "schedule": computes schedule via Cron(run_sch).schedule(...), validates non-None before appending to conf.mySchedules, raises ValueError with specific error log on invalid schedules, and adds a catch-all exception handler for unexpected scheduling errors.

Changes

Cohort / File(s) Summary
Scheduling initialization and validation
server/initialise.py
Replaces direct schedule creation/registration with validated creation: compute schedule, check non-None, append to conf.mySchedules only if valid; raise ValueError and log [ERROR] for invalid input; add generic exception handler and logging for unexpected failures.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Init as initialise.py
  participant Cron as Cron(run_sch)
  participant Conf as conf.mySchedules
  participant Log as Logger

  rect rgb(240,245,255)
  note over Init: RUN == "schedule"
  Init->>Cron: schedule(plugin, offset, ...)
  Cron-->>Init: scheduleObj or None / throws
  end

  alt Valid scheduleObj
    Init->>Conf: append(scheduleObj)
    Log-->>Init: info: schedule set
  else Invalid (None)
    Init->>Log: [ERROR] Invalid schedule
    Init->>Init: raise ValueError
  else Exception thrown
    Init->>Log: [ERROR] Unexpected scheduling error
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at the clockwork chime,
Now schedules hop in validated time.
If None appears, I thump—abort the run!
With guarded paths, the race is neatly won.
Logs nibble errors; carrots for success—yum!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the primary change of improving the scheduler setup's robustness against invalid scheduling inputs. It directly reflects the enhanced error handling and validation logic introduced in the pull request. It is clear, concise, and actionable for anyone scanning the commit history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c8fa55 and 4fef4a7.

📒 Files selected for processing (1)
  • server/initialise.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{server/**/*.py,front/plugins/*/*.py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

{server/**/*.py,front/plugins/*/*.py}: Never hardcode ports or secrets; define settings once (core ccd() or plugin manifest) and read via get_setting_value() everywhere
Use logger.mylog(level, [message]) with levels none/minimal/verbose/debug/trace; keep verbose output concise unless debugging
Reuse helpers in helper.py (timeNowTZ, normalize_mac, sanitizers) and prefer server/db/db_helper.py over raw SQL in new paths

Files:

  • server/initialise.py
🧬 Code graph analysis (1)
server/initialise.py (3)
front/js/common.js (1)
  • tz (369-369)
server/scheduler.py (1)
  • schedule_class (8-43)
server/logger.py (1)
  • mylog (86-89)
🪛 Ruff (0.13.1)
server/initialise.py

377-377: Abstract raise to an inner function

(TRY301)


377-377: Avoid specifying long messages outside the exception class

(TRY003)


380-380: Do not catch blind exception: Exception

(BLE001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: docker_dev
🔇 Additional comments (1)
server/initialise.py (1)

371-382: Schedule guard looks good

The defensive Cron initialization and targeted logging keep bad expressions from polluting conf.mySchedules while still surfacing the root cause—nice quality-of-life improvement for ops.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jokob-sk jokob-sk merged commit 1fc015f into netalertx:main Sep 27, 2025
4 checks passed
@ingoratsdorf ingoratsdorf deleted the scheduler-fix branch September 28, 2025 02:34
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