Skip to content

Rework webhook#856

Open
dmamelin wants to merge 8 commits into
custom-components:masterfrom
dmamelin:rework-webhook
Open

Rework webhook#856
dmamelin wants to merge 8 commits into
custom-components:masterfrom
dmamelin:rework-webhook

Conversation

@dmamelin

@dmamelin dmamelin commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

This PR grew out of the discussion on #843: the consensus there was to split @webhook_handler out as a new decorator (rather than overloading @webhook_trigger with a sets_http_response_code flag), keep @webhook_trigger backward compatible, and integrate the new decorator tightly with the existing decorator pipeline instead of growing a parallel codepath. This PR is one alternative implementation; see "Relationship to #852" below for how it differs from @JayNewstrom's follow-up.

@webhook_handler("create_thing")
def create(payload):
    if "name" not in payload:
        return 400, {"error": "name is required"}
    return 201, {"id": make_thing(payload["name"])}
return response
None (or no return) 200, empty body
str 200, text body
bytes 200, raw body
dict / list 200, JSON body
(int_status, body) tuple int_status, body mapped recursively
aiohttp.web.Response as-is
anything else 500 + warning log

Status codes generated by the decorator itself:

  • 400 — malformed request body
  • 403 — str_expr guard evaluated falsy
  • 500 — uncaught exception in the function (also surfaced via pyscript's normal exception log)
  • 503 — call canceled by another decorator's guard (@task_unique, @state_active)
  • 504 — function didn't return within timeout seconds (default 10)

Kwargs (webhook_id, str_expr, local_only, methods, kwargs) match @webhook_trigger; timeout is new. Only one handler per webhook_id.

Changes to @webhook_trigger

  • methods is normalized to a set, so methods=["GET", "POST"] and methods=["POST", "GET"] are equivalent.
  • default for methods is now explicitly {"POST", "PUT"}.
  • two @webhook_trigger on the same webhook_id with different local_only or methods now raise at registration. Same webhook_id with matching settings is still allowed (the documented multi-trigger case).

Infrastructure changes

  • CallResultHandlerDecorator gains handle_call_exception(data, exc) and handle_call_canceled(data). Defaults forward to handle_call_result(data, None). This makes the success / exception / canceled trichotomy a property of the protocol rather than something each result handler has to bolt on. @webhook_handler overrides both to surface 500/503.
  • DecoratorManager.safe_await(coro) — awaits a coroutine and routes exceptions through handle_exception rather than propagating. Used wherever a buggy extension point shouldn't break sibling work.
  • AutoKwargsDecorator now collects annotations across the MRO (needed by @webhook_handler, which inherits via a mixin chain).

Comparison with #852

  • Richer return-value mapping, 852 supports None / int / Response. This PR also handles str (text body), bytes (raw body), dict/list (JSON body), and (int_status, body) tuples.
  • Bare int deliberately not supported. 852 maps int to a status code with an empty body, so return 200 and return 42 both succeed (silently dropping any "data" interpretation). This PR rejects bare int and returns 500 with a warning — return 200, None and (status, body) tuples are the explicit way to set a status.The cost is one extra character; the benefit is that ambiguous returns don't silently do the wrong thing.
  • timeout kwarg. 852 awaits the function without a timeout — a stuck pyscript task blocks the HTTP request indefinitely. This PR caps it (default 10s, 504 on expiry).
  • Simpler error/cancellation path in the protocol. Both PRs add handle_call_exception to CallResultHandlerDecorator. 852 then handles cancellation by passing CancelledError through the same hook, plus a notified flag and a try/except/finally in _call to make sure handlers get exactly one notification. This PR uses straight if/else paths in _call and a separate handle_call_canceled hook, so cancellation stays a distinct event (503 vs. 500) without flags or finally blocks.

PyscriptFixture

A new pytest fixture (tests/conftest.py) that replaces the ad-hoc setup_script + notify_q + MockOpen boilerplate. I've been mulling this fixture for a while — early drafts have been sitting on a personal branch for some time without a concrete use case to anchor them. The webhook work finally provides that anchor: enough new tests, with non-trivial assertions on values and exceptions, to justify landing the fixture with real examples rather than as a speculative cleanup PR.

Tests now read like:

async def test_thing(pyscript, hass):
    await pyscript.start("""
@webhook_trigger("hook")
def f(payload):
    pyscript.done = payload
""")
    await webhook.async_handle_webhook(hass, "hook", _request(body=b"..."))
    await pyscript.wait_done({"hello": "world"})

Setup:

  • await pyscript.start(source) — single-file shortcut, mounts as hello.py under the pyscript folder.
  • add_file(name, content) / add_files({...}) for multi-file layouts. glob and isfile mocks are wired automatically.
  • pyscript.config, pyscript.yaml_config — mutate before start().
  • pyscript.now — single datetime or list (consumed sequentially as pyscript reads dt_now).

Observation:

  • pyscript.done / pyscript.done2 asyncio.Queue fed automatically by pyscript.done = ... writes in scripts.
  • wait_done(expected) / wait_done2(expected) — await with 4s default timeout. Non-string expected is compared after ast.literal_eval.
  • pyscript.exceptions — fed by a monkeypatch of AstEval.log_exception.
  • wait_exception(expected_type, match=...) — type + substring match.

Strict teardown: asserts the queues(done, done2, exceptions) are empty at end of test. Tests that intentionally produce values or errors must consume them explicitly — silent passes that masked a missing assertion are caught immediately.

The fixture was designed with the existing tests in mind. All of them can be migrated to it. This should improve convenience, readability, and safety thanks to strict teardown.

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.

1 participant