You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
**Recommendation**: Yes, preserve this. It's a good UX — framework errors (rate limit exceeded, invalid config, etc.) produce clean log lines. Unexpected errors get full tracebacks for debugging. The executor can check `isinstance(exc, HassetteError)` to choose the log level.
79
+
`HassetteError` → `logger.error("...")` (one-line message, no traceback). General `Exception` → `logger.exception("...")` (message + traceback). The executor checks `isinstance(exc, HassetteError)` to choose the log method.
80
80
81
-
### 2. Should the executor use `track_execution()` or own timing directly?
81
+
Framework errors (rate limit exceeded, invalid config, etc.) produce clean log lines. Unexpected errors get full tracebacks for debugging. Good UX worth preserving.
82
82
83
-
**Current state**: `run_job()` delegates to `track_execution()`, `_dispatch()` does inline timing.
83
+
### 2. Executor owns timing directly, not via `track_execution()`
84
84
85
-
**Recommendation**: Executor owns timing directly. `track_execution()` re-raises all exceptions, but the executor needs to swallow non-Cancelled exceptions. Fighting the context manager's semantics isn't worth it. The executor's `_execute_handler()` and `_execute_job()` methods each have a `started = time.monotonic()` and compute duration in the `finally` block.
85
+
**Decision**: Executor owns timing directly.
86
+
87
+
`track_execution()` re-raises all exceptions, but the executor needs to swallow non-Cancelled exceptions. Fighting the context manager's semantics isn't worth it. The executor captures `time.monotonic()` at the start of invocation and computes `duration_ms` in the `finally` block. The wall-clock `Instant` is captured separately by the executor (see [prereq 1](./prereq-01-data-model.md) — "`ExecutionResult` stays monotonic-only" aside).
86
88
87
89
`track_execution()` remains available for user code and other contexts where re-raise-after-capture is the right behavior.
88
90
89
-
### 3. Should CancelledError produce a record?
91
+
### 3. CancelledError produces a record, then re-raises
90
92
91
-
**Current behavior**: `_dispatch()` records cancelled in metrics. `run_job()` records cancelled via `track_execution()`.
93
+
**Decision**: Yes, produce a record with `status="cancelled"`, then re-raise.
92
94
93
-
**Recommendation**: Yes, produce a record with `status="cancelled"`. Then re-raise. The record is queued via `put_nowait()` before the `raise`, so the write queue still gets it.
95
+
The record is queued via `put_nowait()` before the re-raise, so the write queue still gets it. Swallowing `CancelledError` would break asyncio's cancellation machinery. See [prereq 1](./prereq-01-data-model.md) for full `cancelled` status documentation.
94
96
95
-
### 4. Should the executor handle `listener.once` removal?
97
+
### 4. Executor does NOT handle `listener.once` removal
96
98
97
-
**Current behavior**: `_dispatch()` removes one-shot listeners in its `finally` block.
99
+
**Decision**: Keep `listener.once` removal in `_dispatch()`, not the executor.
98
100
99
-
**Recommendation**: No. `listener.once` is bus routing logic, not an execution concern. Keep it in `_dispatch()`:
101
+
`listener.once` is bus routing logic, not an execution concern:
This keeps the executor focused on execution + recording, and the bus focused on listener lifecycle.
110
+
The executor is focused on execution + recording. The bus owns listener lifecycle. Similarly, the executor does not own job rescheduling — that stays in `SchedulerService`.
109
111
110
-
## Deliverable
112
+
## Executor exception contract (summary)
111
113
112
-
A decision doc (this file, updated with final decisions) that defines the executor's exception contract:
114
+
```
115
+
CancelledError → record status="cancelled" → RE-RAISE
116
+
DependencyError → record status="error", error_type="DependencyError" → logger.error() → SWALLOW
117
+
HassetteError → record status="error" → logger.error() (clean, no traceback) → SWALLOW
118
+
Exception → record status="error" → logger.exception() (with traceback) → SWALLOW
119
+
```
113
120
114
-
- What it catches, what it swallows, what it re-raises
115
-
- How it logs different exception types
116
-
- What status values it produces
117
-
- What it does NOT own (listener lifecycle, job rescheduling)
121
+
**What the executor owns**: invocation, timing, record creation, error classification, logging, error hooks (future).
122
+
123
+
**What the executor does NOT own**: listener lifecycle (`once` removal), job rescheduling, topic routing, DI resolution.
124
+
125
+
## Deliverable
118
126
119
-
This directly feeds into the `CommandExecutor` implementation and the test cases needed to verify behavioral equivalence.
127
+
This file (decisions finalized). Feeds directly into the `CommandExecutor` implementation and the test cases needed to verify behavioral equivalence with the current `_dispatch()` and `run_job()` methods.
0 commit comments