luc-pimentel opened a new pull request, #76407:
URL: https://github.com/apache/airflow/pull/76407

   A `KeyError` or `AttributeError` that goes through `BaseSerialization` comes 
back with the wrong args: `KeyError("boom")` becomes `KeyError(('boom',))`. The 
round trip gives the same result on 3.3.2, 3.4.0rc2 and main. This fixes #69743.
   
   `BaseSerialization.serialize` in `serialized_objects.py` stored 
`[var.args]`, and deserializing calls `exc_cls(*args)`, so the whole tuple 
became the one argument. It now stores the args flat, as the `AirflowException` 
branch above it does.
   
   The impact is small. Of the paths I ran (`defer()` kwargs, trigger events, 
plain and mapped `op_args`, `StartTriggerArgs`), a task only sees this through 
`StartTriggerArgs`, and it still succeeds, with the wrong args. Some provider 
sensors and operators use that route, but I found no caller in the repo that 
puts an exception in it, and I know of no affected Dag.
   
   These are the values the task or trigger received on main, running `airflow 
standalone` in Breeze with SQLite.
   
   | In `StartTriggerArgs` | Without the fix | With the fix |
   |---|---|---|
   | `next_kwargs={"err": KeyError("boom")}` | `KeyError(('boom',))` | 
`KeyError('boom')` |
   | `trigger_kwargs={"err": KeyError("boom")}` | `KeyError(('boom',))` | 
`KeyError('boom')` |
   | `next_kwargs={"err": "plain string"}` | `'plain string'` | `'plain 
string'` |
   
   Deserialize does not change, because a stored `[("a",)]` could also be a 
real `KeyError(("a",))`. With hand-built nodes, one stored nested still reads 
back nested, and 3.3.2, 3.4.0rc2 and main read a flat one correctly. A Dag 
holding such an exception gets one new Dag version after the change and then 
stays at it.
   
   The existing round-trip test pinned the nested shape, with a comment that 
says it was "asserted as it is rather than as it ought to be". That comment was 
a side remark, not a request for this fix, but the test has to change with it. 
Its 8 cases now fail without the fix and pass with it. `prek` (pre-commit 
stage) and the test commands from `breeze verify` also pass. I did not run the 
manual prek stage, the docs build, or Postgres and MySQL.
   
   Serialization is performance sensitive, so I timed it in Breeze (three 
alternating runs of each, medians).
   
   | Call | Without the fix | With the fix |
   |---|---|---|
   | `serialize(KeyError("boom"))` | 6.83 us | 5.29 us |
   | `serialize(dict)` (control) | 3.80 us | 3.77 us |
   
   Credit to #69752 and #69766, which proposed the same one-line change.
   
   closes: #69743
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes - Claude Code (claude-opus-5-5)
   
   Generated-by: Claude Code (claude-opus-5-5) following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to