fix(webhook): avoid synchronous SQLite writes in async event route #95

New issue

Open

opened 2026-09-25 09:59:12 +00:00 by gabogg · 1 comment

gabogg commented

2026-09-25 09:59:12 +00:00

Owner

Problem

PR #94 review found that the async /api/event/webhook route calls DoorStateManager.handle_webhook_event, which performs synchronous SQLite writes (including door transition persistence). Those writes block the event loop during webhook bursts. This predates #94; its new transition write increases the work done on that path.

Approach

Move the webhook persistence and broadcast path to async repository methods, or isolate all synchronous work in a worker thread and schedule the WebSocket emission back on the main loop. Preserve the existing event ordering, one-broadcast-per-persisted-transition rule, and error handling.

Verification

Add an ASGI webhook integration test with real SQLite and a concurrent event-loop task to prove webhook persistence does not block the loop. Keep external HikCentral calls stubbed.

Found in PR #94 review.

## Problem PR #94 review found that the async `/api/event/webhook` route calls `DoorStateManager.handle_webhook_event`, which performs synchronous SQLite writes (including door transition persistence). Those writes block the event loop during webhook bursts. This predates #94; its new transition write increases the work done on that path. ## Approach Move the webhook persistence and broadcast path to async repository methods, or isolate all synchronous work in a worker thread and schedule the WebSocket emission back on the main loop. Preserve the existing event ordering, one-broadcast-per-persisted-transition rule, and error handling. ## Verification Add an ASGI webhook integration test with real SQLite and a concurrent event-loop task to prove webhook persistence does not block the loop. Keep external HikCentral calls stubbed. Found in PR #94 review.

gabogg referenced this issue

2026-09-25 09:59:44 +00:00

feat(telemetry): broadcast door state transitions as their own message (#55) #94

gabogg added the

ready-for-agent

label

2026-09-25 10:04:51 +00:00

gabogg referenced this issue

2026-09-25 10:28:23 +00:00

feat(telemetry): broadcast door state transitions as their own message (#55) #94

~~gabogg referenced this issue 2026-09-25 18:14:27 +00:00~~

fix(doors): async webhook persistence and transition cleanup (#95, #102) #112

gabogg referenced this issue

2026-09-25 18:20:37 +00:00

fix(doors): async webhook persistence and transition cleanup (#95, #102) #112

gabogg referenced this issue from a commit

2026-09-25 18:33:44 +00:00

fix(doors): preserve webhook broadcasts after cancellation

gabogg referenced this issue

2026-09-25 18:43:42 +00:00

fix(doors): async webhook persistence and transition cleanup (#95, #102) #112

gabogg commented

2026-09-25 18:43:43 +00:00

Author

Owner

PR #112 second-pass review follow-ups (minor; keep this issue open after merge):

Threaded overview can race async polling. app/controllers/webhook_controller.py:36 now builds the overview with asyncio.to_thread. get_door_overview() iterates self.doors.items() while sync_doors_async() can insert a newly discovered door into that dict. A deterministic reproduction raises RuntimeError: dictionary changed size during iteration; the webhook then returns {"code":"1"} and omits its door_event overview, although its transition has already been persisted and broadcast. Snapshot the door cache or serialize overview construction with cache writes, and add a concurrent polling/webhook test.
Immediate cardholder lookup is deferred. The threaded overview calls _cardholder_name(), whose person_directory.request() requires a running event loop and silently returns False in the worker thread. A card-only webhook therefore does not start lookup during its immediate overview. The periodic monitor can start it later. Schedule the lookup on the event loop and cover the ASGI webhook route with a card-only event.

The main #95 event-loop responsiveness and cancellation/broadcast requirements are covered by PR #112.

PR #112 second-pass review follow-ups (minor; keep this issue open after merge): - **Threaded overview can race async polling.** `app/controllers/webhook_controller.py:36` now builds the overview with `asyncio.to_thread`. `get_door_overview()` iterates `self.doors.items()` while `sync_doors_async()` can insert a newly discovered door into that dict. A deterministic reproduction raises `RuntimeError: dictionary changed size during iteration`; the webhook then returns `{"code":"1"}` and omits its `door_event` overview, although its transition has already been persisted and broadcast. Snapshot the door cache or serialize overview construction with cache writes, and add a concurrent polling/webhook test. - **Immediate cardholder lookup is deferred.** The threaded overview calls `_cardholder_name()`, whose `person_directory.request()` requires a running event loop and silently returns `False` in the worker thread. A card-only webhook therefore does not start lookup during its immediate overview. The periodic monitor can start it later. Schedule the lookup on the event loop and cover the ASGI webhook route with a card-only event. The main #95 event-loop responsiveness and cancellation/broadcast requirements are covered by PR #112.

gabogg referenced this issue

2026-09-25 18:44:15 +00:00

fix(doors): async webhook persistence and transition cleanup (#95, #102) #112

gabogg referenced this issue from a commit

2026-09-25 18:45:11 +00:00

Merge pull request 'fix(doors): async webhook persistence and transition cleanup (#95, #102)' (#112) from fix/door-transition-async-cleanup into master