Skip to content

DM-54689: Tighten webhook handler — typed headers, single not-configured raise site, consistent log key #266

@jonathansick

Description

@jonathansick

Metadata

Field Value
Parent PRD #232
Jira Key DM-54689
Jira URL https://rubinobs.atlassian.net/browse/DM-54689
Task Order 21
Type AFK
Blocked by None
Parallel with #267, #268, #269
Branch tickets/DM-54689-github-webhook

Origin

Review of PR #264

What to build

Three small polishings of src/docverse/handlers/webhooks/github.py:

  • _lowercase_headers (currently lines 119-129) accepts headers: object and guards .items() with hasattr. FastAPI's request.headers is always a starlette.datastructures.Headers instance, so the guard is dead code and the parameter type is too loose.
  • The dispatch path (lines 90-97) calls factory.get_github_webhook_secret() and factory.create_push_event_processor() in sequence inside one try / except GitHubAppNotConfiguredError. Both can raise the same error from the Factory — collapse to a single accessor so there is one raise site.
  • _handle_push (lines 56-60) logs delivery_id=event.delivery_id while the surrounding handler binds github_delivery_id via context.rebind_logger (lines 110-112). Two field names for the same value hurts greppability.

Approach sketch

  • Type _lowercase_headers parameter as Mapping[str, str] (vendor-neutral; starlette.datastructures.Headers satisfies it). Drop if hasattr(headers, "items"). The body becomes one comprehension.
  • Add a Factory accessor (e.g. create_webhook_dispatch()) that returns (secret: str, processor: PushEventProcessor) and raises GitHubAppNotConfiguredError once. The handler's try block then wraps a single call.
  • Drop delivery_id=event.delivery_id from the info call. The bound github_delivery_id already shows up in the log record.

Acceptance criteria

  • _lowercase_headers parameter is Mapping[str, str]; no hasattr guard; nox -s typing clean.
  • Webhook dispatch raises GitHubAppNotConfiguredError from a single Factory call site (not two adjacent calls).
  • Processed push webhook log line emits github_delivery_id only (no duplicate delivery_id key).
  • Existing webhook handler tests still pass without modification.

Metadata

Metadata

Assignees

No one assigned

    Labels

    prd-taskImplementation task from a PRD

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions