Fix AwaitMessageTrigger crash on tombstone messages when apply_function is unset#69665
Open
TimurRakhmatullin86 wants to merge 1 commit into
Open
Conversation
…on is unset Signed-off-by: Timur Rakhmatullin <174210871+TimurRakhmatullin86@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
AwaitMessageTriggeris used without anapply_function(supported since #55437), the trigger callsmessage.value().decode("utf-8")on every polled message. For a tombstone message —value()isNone, a routine occurrence on log-compacted topics where a null value is the deletion marker — this raisesAttributeError: 'NoneType' object has no attribute 'decode', crashing the trigger and failing the deferred task.This PR treats a tombstone the same way as any other non-matching message: no event is emitted, the offset is committed (when
commit_offset=True, matching the existing non-matching path so the tombstone is not re-read forever after a restart), and the trigger keeps polling.Why skip rather than emit: the trigger's contract is already truthiness-based (
if event:), so aNonepayload cannot be yielded on this path today, and aTriggerEvent(None)would carry no usable payload downstream. Users who need to react to tombstones can already do so viaapply_function, which receives the raw message (includingvalue() is None) and can map it to any truthy payload. Surfacing tombstones from the no-apply_functionconvenience path would be a separate opt-in feature; crashing on them is a bug.Tests:
test_trigger_run_tombstone_message_keeps_polling— regression test:poll()returns a null-value message; asserts the trigger neither raises nor yields and commits the tombstone offset. Fails onmainwith the exactAttributeErrorabove.test_trigger_run_without_apply_function_yields_message_value— asserts the happy path still yields the decoded payload.MockedMessagenow has avalue()method: theapply_function=Nonecase oftest_trigger_run_goodpreviously passed for the wrong reason — the mock lackedvalue(), so the task finished via this sameAttributeError, satisfyingtask.done() is True.No behavior change for non-null messages or when
apply_functionis set. Empty (b"") payloads were already treated as non-events by theif event:check — unchanged.