Scheduler+API Bug - if an invalid timezone was set (through edit of watch or API) it could have crashed the scheduler, Added timezone to the official API docs (#4350)

* Scheduler+API Bug - if an invalid timezone was set (through edit of watch or API) it could have crashed the scheduler, Added `timezone` to the official API docs

* adding missing files
This commit is contained in:
dgtlmoon
2026-09-01 17:00:03 +02:00
committed by GitHub
parent 2b0c5e24e6
commit dce6f23cf9
10 changed files with 949 additions and 18 deletions
+46
View File
@@ -18,6 +18,42 @@ from ..notification import valid_notification_formats
from ..notification.handler import newline_re
def validate_time_schedule_limit(json_data):
"""
Validate the optional timezone inside time_schedule_limit.
The edit form runs validateTimeZoneName on this field, but the API did not -
the OpenAPI schema for time_schedule_limit does not declare a `timezone`
property at all and does not set additionalProperties:false, so any string
passed straight through and was stored.
That matters because the scheduler resolves it with arrow.now(tz), which
raises on an unknown zone. Before the ticker thread was hardened, a single
authenticated PUT with a bogus timezone stopped scheduling for EVERY watch
on the instance. It is now contained to the one watch, but that watch would
still silently never be checked again, so reject it at the boundary.
Returns None if valid, or an error message string if invalid.
"""
schedule = json_data.get('time_schedule_limit')
if not isinstance(schedule, dict):
return None
tz_name = schedule.get('timezone')
if not tz_name:
return None
if not isinstance(tz_name, str):
return "time_schedule_limit.timezone must be a string IANA timezone name, e.g. 'Europe/Berlin'."
from zoneinfo import available_timezones
if tz_name.strip() not in available_timezones():
return (f"time_schedule_limit.timezone '{tz_name}' is not a valid timezone name. "
f"Use an IANA name such as 'Europe/Berlin' or 'UTC'.")
return None
def validate_time_between_check_required(json_data):
"""
Validate that at least one time interval is specified when not using default settings.
@@ -159,6 +195,11 @@ class Watch(Resource):
if validation_error:
return validation_error, 400
# An invalid timezone here makes the watch permanently unschedulable
validation_error = validate_time_schedule_limit(request.json)
if validation_error:
return validation_error, 400
# Validate notification_urls if provided
if 'notification_urls' in request.json:
from wtforms import ValidationError
@@ -488,6 +529,11 @@ class CreateWatch(Resource):
if validation_error:
return validation_error, 400
# An invalid timezone here makes the watch permanently unschedulable
validation_error = validate_time_schedule_limit(json_data)
if validation_error:
return validation_error, 400
# Validate notification_urls if provided
if 'notification_urls' in json_data:
from wtforms import ValidationError
+20 -9
View File
@@ -8,7 +8,7 @@ from jinja2 import Environment, FileSystemLoader
from changedetectionio.store import ChangeDetectionStore
from changedetectionio.auth_decorator import login_optionally_required
from changedetectionio.time_handler import is_within_schedule
from changedetectionio.time_handler import default_timezone_name, is_within_schedule
from changedetectionio import worker_pool
from changedetectionio.llm.evaluator import get_llm_config as _get_llm_config
@@ -253,13 +253,14 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
watch = datastore.data['watching'].get(uuid)
if watch.get('time_between_check_use_default'):
time_schedule_limit = datastore.data['settings']['requests'].get('time_schedule_limit', {})
time_schedule_limit = datastore.data['settings']['requests'].get('time_schedule_limit') or {}
else:
time_schedule_limit = watch.get('time_schedule_limit')
time_schedule_limit = watch.get('time_schedule_limit') or {}
tz_name = time_schedule_limit.get('timezone')
if not tz_name:
tz_name = datastore.data['settings']['application'].get('scheduler_timezone_default', os.getenv('TZ', 'UTC').strip())
tz_name = default_timezone_name(
time_schedule_limit.get('timezone')
or datastore.data['settings']['application'].get('scheduler_timezone_default')
)
if time_schedule_limit and time_schedule_limit.get('enabled'):
try:
@@ -267,9 +268,12 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
default_tz=tz_name
)
except Exception as e:
# Only decides whether to queue an immediate recheck — the watch is
# already saved by this point. Returning a bare `False` from a view
# made Flask raise TypeError and the save appeared to fail with a 500.
logger.error(
f"{uuid} - Recheck scheduler, error handling timezone, check skipped - TZ name '{tz_name}' - {str(e)}")
return False
is_in_schedule = False
#############################
if not datastore.data['watching'][uuid].get('paused') and is_in_schedule:
@@ -469,15 +473,22 @@ def construct_blueprint(datastore: ChangeDetectionStore, update_q, queuedWatchMe
uuid = request.args.get('uuid','')
if datastore.data["watching"].get(uuid):
# Build the new list and REBIND it. Appending in place bypasses
# watch_base.__setitem__, so the watch is never flagged as edited and
# the "content unchanged since last check" skip stays active — the new
# ignore_text would then not take effect until the page changed on its
# own. Assigning the key marks the watch edited and forces reprocessing.
ignore_text = list(datastore.data["watching"][uuid]['ignore_text'])
if mode == 'exact':
for l in selection.splitlines():
datastore.data["watching"][uuid]['ignore_text'].append(l.strip())
ignore_text.append(l.strip())
elif mode == 'digit-regex':
for l in selection.splitlines():
# Replace any series of numbers with a regex
s = re.escape(l.strip())
s = re.sub(r'[0-9]+', r'\\d+', s)
datastore.data["watching"][uuid]['ignore_text'].append('/' + s + '/')
ignore_text.append('/' + s + '/')
datastore.data["watching"][uuid]['ignore_text'] = ignore_text
# Save the updated ignore_text
datastore.data["watching"][uuid].commit()
+23 -4
View File
@@ -44,7 +44,8 @@ from changedetectionio import __version__
from changedetectionio import queuedWatchMetaData
from changedetectionio.api import Watch, WatchHistory, WatchSingleHistory, WatchHistoryDiff, CreateWatch, Import, SystemInfo, Tag, Tags, Notifications, WatchFavicon, Spec
from changedetectionio.api.Search import Search
from .time_handler import is_within_schedule
from .time_handler import default_timezone_name, is_within_schedule
from .thread_supervisor import start_supervised_thread
from changedetectionio.languages import get_available_languages, get_language_codes, get_flag_for_locale, get_timeago_locale
from changedetectionio.favicon_utils import get_favicon_mime_type
@@ -1115,7 +1116,19 @@ def changedetection_app(config=None, datastore_o=None):
batch_mode = app.config.get('batch_mode', False)
if not batch_mode:
# @todo handle ctrl break
ticker_thread = threading.Thread(target=ticker_thread_check_time_launch_checks, daemon=True, name="TickerThread-ScheduleChecker").start()
# Supervised: if the ticker ever returns or raises it is logged CRITICAL and
# restarted. A bare Thread cannot be restarted once its target returns, and a
# dead ticker means no watch is ever checked again while the process keeps
# looking healthy. Note this keeps a real Thread handle - Thread(...).start()
# returns None, so the old assignment left `ticker_thread` permanently None.
ticker_thread = start_supervised_thread(
target=ticker_thread_check_time_launch_checks,
name="TickerThread-ScheduleChecker",
exit_event=app.config.exit,
# sigshutdown_handler() sets both of these; check both so a restart
# can never race an in-progress shutdown.
is_shutting_down=lambda: bool(getattr(datastore, 'stop_thread', False)),
)
# Start configurable number of notification workers (default 1)
notification_workers = int(os.getenv("NOTIFICATION_WORKERS", "1"))
@@ -1315,7 +1328,9 @@ def ticker_thread_check_time_launch_checks():
time_schedule_limit = watch.get('time_schedule_limit')
scheduler_source = 'watch'
tz_name = datastore.data['settings']['application'].get('scheduler_timezone_default', os.getenv('TZ', 'UTC').strip())
tz_name = default_timezone_name(
datastore.data['settings']['application'].get('scheduler_timezone_default')
)
if time_schedule_limit and time_schedule_limit.get('enabled'):
logger.trace(f"{uuid} Time scheduler - Using scheduler settings from {scheduler_source}")
@@ -1327,9 +1342,13 @@ def ticker_thread_check_time_launch_checks():
logger.trace(f"{uuid} Time scheduler - not within schedule skipping.")
continue
except Exception as e:
# `continue`, never `return` — this runs inside the ticker thread's
# main `while not exit.is_set()` loop, so returning here killed the
# scheduler outright and no watch was ever checked again until
# restart. One watch with a bad schedule must not stop the others.
logger.error(
f"{uuid} - Recheck scheduler, error handling timezone, check skipped - TZ name '{tz_name}' - {str(e)}")
return False
continue
# If they supplied an individual entry minutes to threshold.
threshold = recheck_time_system_seconds if watch.get('time_between_check_use_default') else watch.threshold_seconds()
@@ -0,0 +1,297 @@
#!/usr/bin/env python3
"""
Regression tests for two scheduler/timezone bugs.
BUG 1 - edit page returned HTTP 500
ui/edit.py resolved the default timezone with
.get('scheduler_timezone_default', os.getenv('TZ', 'UTC'))
but App.py initialises that key to None, so the key EXISTS and dict.get()
never uses its fallback. None reached is_within_schedule(), which does
`tz_name.strip()` -> AttributeError -> the handler did `return False` -> Flask
raised TypeError ("view function did not return a valid response ... it was a
bool") -> HTTP 500. The watch was actually saved, so the user saw a 500 on a
successful save.
BUG 2 - ticker thread died silently
Same root cause, but flask_app.py's `return False` was inside the ticker
thread's main loop. One watch with an unresolvable timezone ended the thread
and NO watch was ever checked again until restart, with the process still up.
Run: pytest changedetectionio/tests/test_scheduler_timezone_regressions.py
"""
import time
from flask import url_for
from .util import live_server_setup, wait_for_all_checks, set_original_response
DAYS = ['monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'sunday']
def _schedule_form(url, enabled_days=('monday',)):
data = {
"url": url,
"fetch_backend": "html_requests",
"time_between_check_use_default": "",
"time_between_check-seconds": 1,
"time_schedule_limit-enabled": 'y',
}
for day in DAYS:
data[f"time_schedule_limit-{day}-start_time"] = "00:00"
data[f"time_schedule_limit-{day}-duration-hours"] = 24
data[f"time_schedule_limit-{day}-duration-minutes"] = 0
if day in enabled_days:
data[f"time_schedule_limit-{day}-enabled"] = 'y'
return data
def _full_schedule(enabled=True, timezone=None):
sched = {'enabled': True}
for day in DAYS:
sched[day] = {'enabled': enabled, 'start_time': '00:00',
'duration': {'hours': '24', 'minutes': '00'}}
if timezone is not None:
sched['timezone'] = timezone
return sched
def test_edit_page_saves_when_no_default_timezone_configured(client, live_server, measure_memory_usage, datastore_path):
"""BUG 1: enabling the scheduler with scheduler_timezone_default unset 500'd."""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
# This is the default state from model/App.py - key present, value None.
datastore.data['settings']['application']['scheduler_timezone_default'] = None
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
res = client.post(
url_for("ui.ui_edit.edit_page", uuid=uuid),
data=_schedule_form(test_url),
follow_redirects=True,
)
assert res.status_code == 200, f"expected 200, got {res.status_code}"
assert b"Internal Server Error" not in res.data
assert b"Updated watch." in res.data
# and the schedule really was saved
assert datastore.data['watching'][uuid]['time_schedule_limit']['enabled'] is True
def test_edit_page_saves_with_unresolvable_timezone(client, live_server, measure_memory_usage, datastore_path):
"""A garbage timezone must degrade to 'skip the recheck', not 500."""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
datastore.data['settings']['application']['scheduler_timezone_default'] = 'Not/ARealZone'
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
res = client.post(
url_for("ui.ui_edit.edit_page", uuid=uuid),
data=_schedule_form(test_url),
follow_redirects=True,
)
assert res.status_code == 200, f"expected 200, got {res.status_code}"
assert b"Updated watch." in res.data
def test_ticker_survives_a_watch_with_a_broken_timezone(client, live_server, measure_memory_usage, datastore_path):
"""
BUG 2: one bad watch used to kill the ticker thread, stopping every other
watch. Prove a healthy watch still gets checked alongside a broken one.
"""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
datastore.data['settings']['application']['scheduler_timezone_default'] = None
broken_uuid = datastore.add_watch(url=test_url)
healthy_uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
# Broken watch: scheduler on, timezone that arrow cannot resolve
datastore.data['watching'][broken_uuid]['time_between_check_use_default'] = False
datastore.data['watching'][broken_uuid]['time_schedule_limit'] = _full_schedule(
timezone='Not/ARealZone'
)
# Healthy watch: no schedule limit, short interval so the ticker must pick it up
datastore.data['watching'][healthy_uuid]['time_between_check_use_default'] = False
datastore.data['watching'][healthy_uuid]['time_between_check'] = {
'weeks': None, 'days': None, 'hours': None, 'minutes': None, 'seconds': 2
}
before = datastore.data['watching'][healthy_uuid]['last_checked']
# Give the ticker several passes over the broken watch
deadline = time.time() + 30
while time.time() < deadline:
if datastore.data['watching'][healthy_uuid]['last_checked'] != before:
break
time.sleep(0.5)
assert datastore.data['watching'][healthy_uuid]['last_checked'] != before, (
"the healthy watch was never rechecked - the ticker thread most likely "
"died on the broken watch's timezone"
)
def test_api_rejects_invalid_timezone_on_update(client, live_server, measure_memory_usage, datastore_path):
"""
The edit form runs validateTimeZoneName, but the API did not - the OpenAPI
schema for time_schedule_limit declares no `timezone` property and does not
set additionalProperties:false, so any string was accepted and stored.
A bogus zone makes arrow.now(tz) raise inside the scheduler.
"""
import json
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
api_key = datastore.data['settings']['application'].get('api_access_token')
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
res = client.put(
url_for("watch", uuid=uuid),
headers={'x-api-key': api_key, 'content-type': 'application/json'},
data=json.dumps({'time_schedule_limit': _full_schedule(timezone='Not/ARealZone')}),
)
assert res.status_code == 400, f"expected 400, got {res.status_code}: {res.data}"
assert b'not a valid timezone' in res.data.lower()
stored = datastore.data['watching'][uuid]['time_schedule_limit']
assert stored.get('timezone') != 'Not/ARealZone', "invalid timezone was persisted anyway"
def test_api_accepts_valid_timezone_on_update(client, live_server, measure_memory_usage, datastore_path):
"""The validation must not block legitimate zones."""
import json
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
api_key = datastore.data['settings']['application'].get('api_access_token')
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
for tz in ('Europe/Berlin', 'UTC', 'Pacific/Kiritimati'):
res = client.put(
url_for("watch", uuid=uuid),
headers={'x-api-key': api_key, 'content-type': 'application/json'},
data=json.dumps({'time_schedule_limit': _full_schedule(timezone=tz)}),
)
assert res.status_code == 200, f"{tz} rejected: {res.data}"
assert datastore.data['watching'][uuid]['time_schedule_limit']['timezone'] == tz
# omitting the timezone entirely stays valid
res = client.put(
url_for("watch", uuid=uuid),
headers={'x-api-key': api_key, 'content-type': 'application/json'},
data=json.dumps({'time_schedule_limit': _full_schedule()}),
)
assert res.status_code == 200, res.data
def test_api_rejects_invalid_timezone_on_create(client, live_server, measure_memory_usage, datastore_path):
"""Same guard on POST /watch, otherwise it is just a different door."""
import json
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
api_key = datastore.data['settings']['application'].get('api_access_token')
res = client.post(
url_for("createwatch"),
headers={'x-api-key': api_key, 'content-type': 'application/json'},
data=json.dumps({'url': test_url,
'time_schedule_limit': _full_schedule(timezone='Bogus/Zone')}),
)
assert res.status_code == 400, f"expected 400, got {res.status_code}: {res.data}"
assert b'not a valid timezone' in res.data.lower()
def test_ignore_text_via_selection_marks_watch_edited(client, live_server, measure_memory_usage, datastore_path):
"""
The selection UI appended to ignore_text in place. That bypasses
watch_base.__setitem__, so was_edited stayed False and the
'content unchanged since last check' skip stayed active - the new
ignore_text would not take effect until the page changed on its own.
"""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
watch = datastore.data['watching'][uuid]
watch.reset_watch_edited_flag()
assert watch.was_edited is False, "precondition: flag should start clear"
res = client.post(
url_for("ui.ui_edit.highlight_submit_ignore_url", uuid=uuid),
data={'mode': 'exact', 'selection': 'Which is across multiple lines'},
follow_redirects=True,
)
assert res.status_code == 200
watch = datastore.data['watching'][uuid]
assert 'Which is across multiple lines' in watch['ignore_text'], "the text was not stored"
assert watch.was_edited is True, (
"watch was not flagged as edited - the new ignore_text will not be applied "
"until the page content changes on its own"
)
def test_ignore_text_digit_regex_marks_watch_edited(client, live_server, measure_memory_usage, datastore_path):
"""Same for the digit-regex branch."""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
datastore.data['watching'][uuid].reset_watch_edited_flag()
res = client.post(
url_for("ui.ui_edit.highlight_submit_ignore_url", uuid=uuid),
data={'mode': 'digit-regex', 'selection': 'Updated 1234 times'},
follow_redirects=True,
)
assert res.status_code == 200
watch = datastore.data['watching'][uuid]
assert any(t.startswith('/') and t.endswith('/') for t in watch['ignore_text']), \
f"expected a regex entry, got {watch['ignore_text']}"
assert watch.was_edited is True
def test_ignore_text_appends_to_existing(client, live_server, measure_memory_usage, datastore_path):
"""Rebinding must preserve entries already there, not replace them."""
set_original_response(datastore_path=datastore_path)
test_url = url_for('test_endpoint', _external=True)
datastore = live_server.app.config['DATASTORE']
uuid = datastore.add_watch(url=test_url)
wait_for_all_checks(client)
datastore.data['watching'][uuid]['ignore_text'] = ['already here']
client.post(
url_for("ui.ui_edit.highlight_submit_ignore_url", uuid=uuid),
data={'mode': 'exact', 'selection': 'line one\nline two'},
follow_redirects=True,
)
ignore_text = datastore.data['watching'][uuid]['ignore_text']
assert 'already here' in ignore_text, "existing entries were lost"
assert 'line one' in ignore_text
assert 'line two' in ignore_text
@@ -0,0 +1,175 @@
#!/usr/bin/env python3
"""
Regression tests for the scheduler's default-timezone resolution.
The bug: model/App.py initialises 'scheduler_timezone_default' to None, so the
key ALWAYS EXISTS with a None value. Call sites used
settings['application'].get('scheduler_timezone_default', os.getenv('TZ', 'UTC'))
which never reaches its fallback, because dict.get() only substitutes the
default when the key is ABSENT - not when its value is None. The resulting None
flowed into is_within_schedule(), which does `tz_name.strip()` and raised
AttributeError. That took out the edit page (HTTP 500) and, worse, the ticker
thread (which did `return False` inside its main loop and stopped scheduling
every watch until restart).
Run: python3 -m unittest changedetectionio.tests.unit.test_scheduler_timezone_resolution
"""
import os
import unittest
from unittest.mock import patch
from changedetectionio.time_handler import default_timezone_name, is_within_schedule
class TestDefaultTimezoneName(unittest.TestCase):
def test_none_falls_back_to_utc(self):
"""The exact bug: a present-but-None setting must not stay None."""
with patch.dict(os.environ, {}, clear=False):
os.environ.pop('TZ', None)
self.assertEqual(default_timezone_name(None), 'UTC')
def test_empty_string_falls_back(self):
with patch.dict(os.environ, {}, clear=False):
os.environ.pop('TZ', None)
self.assertEqual(default_timezone_name(''), 'UTC')
self.assertEqual(default_timezone_name(' '), 'UTC')
def test_configured_value_wins(self):
self.assertEqual(default_timezone_name('Pacific/Kiritimati'), 'Pacific/Kiritimati')
def test_configured_value_is_stripped(self):
self.assertEqual(default_timezone_name(' Europe/Berlin '), 'Europe/Berlin')
def test_tz_env_used_when_unconfigured(self):
with patch.dict(os.environ, {'TZ': 'Asia/Tokyo'}):
self.assertEqual(default_timezone_name(None), 'Asia/Tokyo')
def test_configured_beats_tz_env(self):
with patch.dict(os.environ, {'TZ': 'Asia/Tokyo'}):
self.assertEqual(default_timezone_name('Europe/Berlin'), 'Europe/Berlin')
def test_never_returns_falsy(self):
with patch.dict(os.environ, {'TZ': ' '}):
self.assertEqual(default_timezone_name(None), 'UTC')
for value in (None, '', ' ', 0, False, []):
self.assertTrue(default_timezone_name(value),
f"default_timezone_name({value!r}) must never be falsy")
def test_reproduces_the_original_crash(self):
"""Passing the un-resolved None straight through still raises - proving
the resolver is what prevents it, not luck elsewhere."""
schedule = {
'enabled': True,
'monday': {'enabled': True, 'start_time': '00:00',
'duration': {'hours': '24', 'minutes': '00'}},
}
with self.assertRaises(AttributeError):
is_within_schedule(time_schedule_limit=schedule, default_tz=None)
# ...and does not raise once resolved
full = {'enabled': True}
for day in ('monday', 'tuesday', 'wednesday', 'thursday',
'friday', 'saturday', 'sunday'):
full[day] = {'enabled': True, 'start_time': '00:00',
'duration': {'hours': '24', 'minutes': '00'}}
self.assertTrue(
is_within_schedule(time_schedule_limit=full,
default_tz=default_timezone_name(None))
)
class TestApiTimezoneValidation(unittest.TestCase):
"""
The edit form runs validateTimeZoneName on time_schedule_limit.timezone, but
the API did not: the OpenAPI schema declared no `timezone` property and did
not set additionalProperties:false, so any string was accepted and stored.
"""
DAYS = ('monday', 'tuesday', 'wednesday', 'thursday',
'friday', 'saturday', 'sunday')
def _payload(self, timezone=..., ):
sched = {'enabled': True}
for day in self.DAYS:
sched[day] = {'enabled': True, 'start_time': '00:00',
'duration': {'hours': '24', 'minutes': '00'}}
if timezone is not ...:
sched['timezone'] = timezone
return {'time_schedule_limit': sched}
def setUp(self):
from changedetectionio.api.Watch import validate_time_schedule_limit
self.validate = validate_time_schedule_limit
def test_valid_zones_accepted(self):
for tz in ('UTC', 'Europe/Berlin', 'America/Los_Angeles', 'Pacific/Kiritimati'):
self.assertIsNone(self.validate(self._payload(tz)), f"{tz} should be valid")
def test_absent_or_empty_timezone_is_fine(self):
self.assertIsNone(self.validate(self._payload())) # key absent
self.assertIsNone(self.validate(self._payload(''))) # empty
self.assertIsNone(self.validate(self._payload(None))) # explicit null
def test_unknown_zone_rejected(self):
err = self.validate(self._payload('Not/ARealZone'))
self.assertIsNotNone(err)
self.assertIn('Not/ARealZone', err)
self.assertIn('not a valid timezone', err.lower())
def test_case_sensitive_like_the_form(self):
"""'utc' is not an IANA name; the form rejects it, so must the API."""
self.assertIsNotNone(self.validate(self._payload('utc')))
def test_non_string_rejected(self):
for bad in (123, 12.5, True, ['UTC'], {'name': 'UTC'}):
self.assertIsNotNone(self.validate(self._payload(bad)),
f"{bad!r} should be rejected")
def test_no_schedule_key_at_all(self):
self.assertIsNone(self.validate({}))
self.assertIsNone(self.validate({'time_schedule_limit': None}))
self.assertIsNone(self.validate({'url': 'https://example.com'}))
def test_non_dict_schedule_is_left_to_openapi(self):
"""Structural type errors are the OpenAPI layer's job, not ours."""
self.assertIsNone(self.validate({'time_schedule_limit': 'nope'}))
self.assertIsNone(self.validate({'time_schedule_limit': []}))
def test_rejected_zone_would_have_crashed_the_scheduler(self):
"""Tie the validation back to the actual failure it prevents."""
payload = self._payload('Not/ARealZone')
self.assertIsNotNone(self.validate(payload))
with self.assertRaises(Exception):
is_within_schedule(time_schedule_limit=payload['time_schedule_limit'],
default_tz='UTC')
class TestApiSpecDocumentsTimezone(unittest.TestCase):
"""The field is real and settable; the published contract must say so."""
def test_timezone_is_declared_in_the_openapi_spec(self):
import os
import yaml
here = os.path.dirname(__file__)
spec_path = os.path.join(here, '..', '..', '..', 'docs', 'api-spec.yaml')
if not os.path.isfile(spec_path):
self.skipTest("api-spec.yaml not present in this layout")
with open(spec_path) as f:
spec = yaml.safe_load(f)
schedule = spec['components']['schemas']['WatchBase']['properties']['time_schedule_limit']
self.assertIn('timezone', schedule['properties'],
"time_schedule_limit.timezone is settable via the form and the "
"API but is missing from the OpenAPI spec")
self.assertEqual(schedule['properties']['timezone']['type'], 'string')
if __name__ == '__main__':
unittest.main()
@@ -0,0 +1,217 @@
#!/usr/bin/env python3
"""
Tests for thread_supervisor.supervise().
Context: the ticker thread had `return False` buried inside its per-watch loop.
One watch with an unresolvable timezone made the whole loop return, the thread
ended, and no watch was ever checked again - with the process still up and
looking healthy. threading.Thread cannot be restarted (start() twice raises
RuntimeError), so recovery has to live inside the thread.
The two behaviours that matter:
- a target that returns or raises is restarted, loudly
- a target that exits during shutdown is NOT restarted
Run: python3 -m unittest changedetectionio.tests.unit.test_thread_supervisor
"""
import threading
import time
import unittest
from changedetectionio.thread_supervisor import supervise, start_supervised_thread
class TestRestartsOnDeath(unittest.TestCase):
def test_target_that_returns_is_restarted(self):
"""The exact ticker bug: a stray `return` must not end the thread."""
exit_event = threading.Event()
calls = []
def flaky():
calls.append('run')
if len(calls) >= 3:
exit_event.set() # let the supervisor finish
return # simulates `return False` in the loop
supervise(flaky, 'flaky', exit_event, max_backoff=0.01, healthy_after=9999)
self.assertEqual(len(calls), 3, "target should have been restarted twice")
def test_target_that_raises_is_restarted(self):
exit_event = threading.Event()
calls = []
def crashy():
calls.append('run')
if len(calls) >= 3:
exit_event.set()
return
raise RuntimeError("boom")
supervise(crashy, 'crashy', exit_event, max_backoff=0.01, healthy_after=9999)
self.assertEqual(len(calls), 3)
def test_restart_reason_is_reported(self):
exit_event = threading.Event()
reasons = []
calls = []
def mixed():
calls.append('run')
if len(calls) == 1:
return # clean but wrong
if len(calls) == 2:
raise ValueError("nope") # crash
exit_event.set()
supervise(mixed, 'mixed', exit_event, max_backoff=0.01, healthy_after=9999,
_on_restart=lambda n, reason: reasons.append(reason))
self.assertEqual(len(reasons), 2)
self.assertIn('returned unexpectedly', reasons[0])
self.assertIn('ValueError: nope', reasons[1])
def test_backoff_grows_then_resets_after_healthy_run(self):
exit_event = threading.Event()
stamps = []
def failing():
stamps.append(time.monotonic())
if len(stamps) >= 4:
exit_event.set()
return
raise RuntimeError("boom")
supervise(failing, 'failing', exit_event, max_backoff=0.08, healthy_after=9999)
gaps = [stamps[i + 1] - stamps[i] for i in range(len(stamps) - 1)]
# 1s doubling is capped at max_backoff, so every gap should hit the cap
for gap in gaps:
self.assertGreaterEqual(gap, 0.05, f"gaps={gaps} - backoff not applied")
def test_does_not_spin_hot(self):
"""A target failing instantly must not be restarted in a tight loop."""
exit_event = threading.Event()
calls = []
start = time.monotonic()
def instant():
calls.append(1)
if len(calls) >= 3:
exit_event.set()
return
raise RuntimeError("instant")
supervise(instant, 'instant', exit_event, max_backoff=0.05, healthy_after=9999)
self.assertGreater(time.monotonic() - start, 0.05,
"supervisor restarted with no delay - would burn CPU")
class TestDoesNotRestartDuringShutdown(unittest.TestCase):
def test_exit_event_stops_restarting(self):
exit_event = threading.Event()
calls = []
def target():
calls.append('run')
exit_event.set() # shutdown requested, then return normally
supervise(target, 'target', exit_event, max_backoff=0.01)
self.assertEqual(len(calls), 1, "must not restart once exit_event is set")
def test_exit_event_set_before_start_never_runs_target(self):
exit_event = threading.Event()
exit_event.set()
calls = []
supervise(lambda: calls.append('run'), 'target', exit_event)
self.assertEqual(calls, [])
def test_secondary_shutdown_flag_stops_restarting(self):
"""sigshutdown_handler also sets datastore.stop_thread - honour it."""
exit_event = threading.Event()
state = {'stop_thread': False}
calls = []
def target():
calls.append('run')
state['stop_thread'] = True # only the secondary flag is set
return
supervise(target, 'target', exit_event,
is_shutting_down=lambda: state['stop_thread'],
max_backoff=0.01)
self.assertEqual(len(calls), 1,
"must not restart when the secondary shutdown flag is set")
self.assertFalse(exit_event.is_set(), "primary flag was never set in this test")
def test_raising_shutdown_predicate_does_not_block_restart(self):
"""A broken predicate must not silently disable recovery."""
exit_event = threading.Event()
calls = []
def target():
calls.append('run')
if len(calls) >= 2:
exit_event.set()
return
def broken():
raise RuntimeError("predicate is broken")
supervise(target, 'target', exit_event, is_shutting_down=broken,
max_backoff=0.01, healthy_after=9999)
self.assertEqual(len(calls), 2, "should still have restarted once")
class TestStartSupervisedThread(unittest.TestCase):
def test_returns_a_real_thread_object(self):
"""Thread(...).start() returns None - that is why ticker_thread was
always None and nothing could check is_alive()."""
exit_event = threading.Event()
started = threading.Event()
def target():
started.set()
exit_event.wait(5)
t = start_supervised_thread(target, 'TestThread', exit_event)
self.addCleanup(exit_event.set)
self.assertIsInstance(t, threading.Thread)
self.assertTrue(started.wait(5), "target did not run")
self.assertTrue(t.is_alive())
self.assertTrue(t.daemon)
self.assertEqual(t.name, 'TestThread')
exit_event.set()
t.join(timeout=5)
self.assertFalse(t.is_alive(), "thread did not exit on shutdown")
def test_thread_survives_a_dying_target(self):
"""End to end: the thread outlives a target that keeps returning."""
exit_event = threading.Event()
calls = []
done = threading.Event()
def target():
calls.append(1)
if len(calls) >= 3:
done.set()
exit_event.wait(5)
return
t = start_supervised_thread(target, 'Dying', exit_event, max_backoff=0.01,
healthy_after=9999)
self.addCleanup(exit_event.set)
self.assertTrue(done.wait(10), f"target was not restarted (calls={len(calls)})")
self.assertTrue(t.is_alive(), "thread died despite supervision")
exit_event.set()
t.join(timeout=5)
if __name__ == '__main__':
unittest.main()
+132
View File
@@ -0,0 +1,132 @@
"""
Keep critical background threads alive.
A threading.Thread cannot be restarted. Once its target returns, the thread is
finished and calling start() again raises RuntimeError("threads can only be
started once"). There is no built-in "respawn on death" option. So recovery has
to be built into the target itself.
This matters because a background loop can die in two ways that look identical
from outside, and both are silent:
- it raises, and the traceback goes nowhere because nobody joins a daemon thread
- it simply `return`s, e.g. a stray `return False` deep inside the loop body
The second is how the ticker thread stopped scheduling every watch in the app:
one watch with an unresolvable timezone hit `return False`, the whole loop
exited, and no watch was ever checked again until the process restarted. Nothing
was logged above ERROR, and the process stayed up and healthy-looking.
supervise() wraps a target so that any exit which is NOT a real shutdown is
logged CRITICAL and the target is re-entered, with exponential backoff so a
target that fails instantly cannot spin the CPU.
"""
import time
from loguru import logger
# Back off 1, 2, 4 ... up to this many seconds between restarts.
DEFAULT_MAX_BACKOFF_SECONDS = 60
# A target that stayed up at least this long is considered to have recovered,
# so its backoff resets rather than continuing to grow.
HEALTHY_RUNTIME_SECONDS = 60
def supervise(target, name, exit_event, is_shutting_down=None,
max_backoff=DEFAULT_MAX_BACKOFF_SECONDS,
healthy_after=HEALTHY_RUNTIME_SECONDS,
_on_restart=None):
"""
Run `target` forever, restarting it if it ever returns or raises.
Never restarts once shutdown has begun - an exiting target is expected then,
and respawning it would fight the shutdown path and delay process exit.
Args:
target: zero-arg callable expected to loop until shutdown
name: thread name, used in log messages
exit_event: threading.Event - the primary shutdown signal
is_shutting_down: optional extra zero-arg predicate returning True during
shutdown. sigshutdown_handler() sets both app.config.exit
and datastore.stop_thread, so pass the latter here to be
certain a restart can never race a shutdown.
max_backoff: ceiling for the restart delay, in seconds
healthy_after: runtime after which the backoff resets to 1s
_on_restart: test hook, called with (restart_count, reason) before each retry
Returns:
None - only once shutdown is signalled
"""
def stopping():
if exit_event.is_set():
return True
if is_shutting_down is not None:
try:
return bool(is_shutting_down())
except Exception:
# A broken predicate must not keep a healthy thread from restarting
return False
return False
backoff = 1
restarts = 0
while not stopping():
started = time.monotonic()
reason = None
try:
target()
# Falling out of the target is only legitimate during shutdown.
if stopping():
break
reason = "returned unexpectedly (a stray `return` inside its loop?)"
logger.critical(
f"{name} {reason} - this thread must run until shutdown. Restarting it."
)
except Exception as e:
if stopping():
break
reason = f"crashed: {type(e).__name__}: {e}"
logger.opt(exception=True).critical(f"{name} {reason} - restarting it.")
ran_for = time.monotonic() - started
backoff = 1 if ran_for >= healthy_after else min(backoff * 2, max_backoff)
restarts += 1
if _on_restart:
_on_restart(restarts, reason)
logger.critical(
f"{name} restart #{restarts} in {backoff}s (previous run lasted {ran_for:.1f}s)"
)
# wait() returns True immediately once shutdown is requested
if exit_event.wait(backoff) or stopping():
break
logger.info(f"{name} supervisor exiting - shutdown requested "
f"(after {restarts} restart(s))")
def start_supervised_thread(target, name, exit_event, is_shutting_down=None, **kwargs):
"""
Start `target` in a daemon thread wrapped in supervise().
Returns:
threading.Thread: the started thread (unlike Thread(...).start(), which
returns None - assigning that to a module global is why nothing could
ever check the ticker thread's is_alive()).
"""
import threading
thread = threading.Thread(
target=supervise,
args=(target, name, exit_event),
kwargs={'is_shutting_down': is_shutting_down, **kwargs},
daemon=True,
name=name,
)
thread.start()
return thread
+23
View File
@@ -1,9 +1,32 @@
import os
from functools import lru_cache
import arrow
from enum import IntEnum
def default_timezone_name(configured=None):
"""
Resolve the scheduler's default IANA timezone name. Never returns None/''.
`scheduler_timezone_default` is initialised to None in model/App.py, so the
key always EXISTS with a None value. That makes the common idiom
settings['application'].get('scheduler_timezone_default', os.getenv('TZ', 'UTC'))
silently return None - dict.get() only falls back when the key is absent,
not when its value is None. The None then reaches is_within_schedule(),
which does `tz_name.strip()` and raises AttributeError.
Args:
configured: the stored setting value (may be None or empty)
Returns:
str: a non-empty timezone name, falling back to $TZ then 'UTC'
"""
return (configured or os.getenv('TZ') or 'UTC').strip() or 'UTC'
class Weekday(IntEnum):
"""Enumeration for days of the week."""
Monday = 0
+11
View File
@@ -569,6 +569,17 @@ components:
$ref: '#/components/schemas/DaySchedule'
sunday:
$ref: '#/components/schemas/DaySchedule'
timezone:
type: string
nullable: true
description: >-
Optional IANA timezone name the schedule is evaluated in, e.g.
"America/Los_Angeles". When empty or omitted, the global
scheduler_timezone_default is used, falling back to $TZ then UTC.
Must be a name recognised by the server's timezone database -
an unknown value is rejected with HTTP 400, because the scheduler
cannot resolve it and the watch would never be checked.
example: America/Los_Angeles
# Conditions (advanced logic)
conditions:
File diff suppressed because one or more lines are too long