Skip to content

FailStage receives null exception when hub callback task is canceled#8064

Merged
Arkatufus merged 2 commits intoakkadotnet:devfrom
MattKotsenas:bugfix/failstage-null-exception
Mar 4, 2026
Merged

FailStage receives null exception when hub callback task is canceled#8064
Arkatufus merged 2 commits intoakkadotnet:devfrom
MattKotsenas:bugfix/failstage-null-exception

Conversation

@MattKotsenas
Copy link
Contributor

@MattKotsenas MattKotsenas commented Feb 27, 2026

Changes

In PartitionHub's consumer PreStart, the OnHubReady callback checked t.IsCanceled || t.IsFaulted but passed t.Exception to FailStage. When a task is canceled (not faulted), t.Exception is null.

I used Copilot to try to understand the implications of this issue. It said:

FailStage(null) causes downstream ports to complete normally instead of failing, silently swallowing the cancellation.

I'm confident that the task handling is incorrect, but I need someone who knows Streams to understand if there's user impact here or not.

In either case, I fixed it by copying BroadcastHub's consumer pattern by wrapping the task through Result.FromTask(), which already handles task cancellation correctly by calling t.GetAwaiter().GetResult() to produce the real TaskCanceledException from the runtime.

Note: enabling #nullable in GraphStage.cs + Hub.cs would catch this at compile time. That's a bigger change though, so will defer that work for the moment.

Checklist

…eled

In PartitionHub's consumer PreStart, the OnHubReady callback checked
t.IsCanceled || t.IsFaulted but passed t.Exception to FailStage.
When a task is canceled (not faulted), t.Exception is null per the
Task contract. FailStage(null) causes downstream ports to complete
normally instead of failing, silently swallowing the cancellation.

Fix: align with BroadcastHub's consumer pattern by wrapping the task
through Result.FromTask(), which already handles task cancellation
correctly by calling t.GetAwaiter().GetResult() to produce the real
TaskCanceledException from the runtime.

Note: enabling #nullable in GraphStage.cs + Hub.cs catches this at
compile time (CS8604 on the FailStage call). That flagged 6 call
sites total, but only this one is a real bug — the other 5 already
go through Result.FromTask. Enabling nullable in these files produces
~11 additional errors and is deferred to a separate effort.
@Aaronontheweb
Copy link
Member

Note: enabling #nullable in GraphStage.cs + Hub.cs would catch this at compile time. That's a bigger change though, so will defer that work for the moment.

yeah @Arkatufus is gradually working on enabling nullability from the bottom up in the dev branch currently

Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Arkatufus Arkatufus enabled auto-merge (squash) March 4, 2026 04:16
@Arkatufus Arkatufus merged commit 950b532 into akkadotnet:dev Mar 4, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants