fix: prevent NRE on transport level gRPC errors

[w1am]

Summary

GetRpcStatus() can return null for transport-level gRPC errors (connection drops, timeouts), but the code assumed it never would. This caused a NullReferenceException that masked the actual error. Now the original RpcException is surfaced instead.

[qodo-code-review]

Review Summary by Qodo

Handle null GetRpcStatus() to prevent NRE on transport errors

🐞 Bug fix

Walkthroughs

Description

• Replace null-forgiving operator with null-coalescing throw in exception factories
• Use null-conditional operator in MultiAppend catch block for safe status access
• Prevent NullReferenceException on transport-level gRPC errors
• Surface original RpcException instead of masking with NRE

Diagram

flowchart LR
  A["RpcException with null Status"] -->|GetRpcStatus()!| B["NullReferenceException"]
  A -->|GetRpcStatus() ?? throw ex| C["Original RpcException surfaced"]
  D["MultiAppend catch block"] -->|ex.GetRpcStatus()!| E["NRE on null status"]
  D -->|ex.GetRpcStatus()?| F["Safe null-conditional access"]

Loading

File Changes

1. src/KurrentDB.Client/Core/Exceptions/AccessDeniedException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Ensures original RpcException is thrown when status is null

src/KurrentDB.Client/Core/Exceptions/AccessDeniedException.cs

---

2. src/KurrentDB.Client/Core/Exceptions/AppendTransactionMaxSizeExceededException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Prevents NRE by throwing original exception when status is unavailable

src/KurrentDB.Client/Core/Exceptions/AppendTransactionMaxSizeExceededException.cs

---

3. src/KurrentDB.Client/Core/Exceptions/NotLeaderException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Handles null status gracefully by rethrowing original exception

src/KurrentDB.Client/Core/Exceptions/NotLeaderException.cs

---

View more (4)

4. src/KurrentDB.Client/Core/Exceptions/StreamTombstonedException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Ensures transport-level errors are not masked with NRE

src/KurrentDB.Client/Core/Exceptions/StreamTombstonedException.cs

---

5. src/KurrentDB.Client/Core/Exceptions/WrongExpectedVersionException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Prevents NRE when RpcException lacks Google.Rpc.Status details

src/KurrentDB.Client/Core/Exceptions/WrongExpectedVersionException.cs

---

6. src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs 🐞 Bug fix +1/-2

Use null-conditional operator in MultiAppend catch

• Replace ex.GetRpcStatus()! with ex.GetRpcStatus()? using null-conditional operator
• Allows null status to fall through to default switch arm which rethrows original exception
• Prevents NullReferenceException on transport-level gRPC failures

src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs

---

7. src/KurrentDB.Client/Streams/MaximumAppendSizeExceededException.cs 🐞 Bug fix +2/-1

Replace null-forgiving with null-coalescing throw

• Replace GetRpcStatus()! with GetRpcStatus() ?? throw ex in FromRpcException factory method
• Handles null status by throwing original RpcException

src/KurrentDB.Client/Streams/MaximumAppendSizeExceededException.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🖥 UI issues (0)   🎨 UX Issues (0)

🐞\ ◔ Observability (1)

1. Rethrow resets stack trace 🐞 ◔

Description

MultiStreamAppendAsync rethrows the caught RpcException via throw <switchExpr> (default arm
returns ex), which resets the exception stack trace to this catch block instead of preserving the
original throw site. This makes transport-level failures harder to debug even though the original
RpcException type/message is preserved.

Code

src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs[R82-85]

+				throw ex.GetRpcStatus()?.GetDetail<ErrorInfo>() switch {
					{ Reason: "STREAM_REVISION_CONFLICT" }         => WrongExpectedVersionException.FromRpcException(ex),
					{ Reason: "STREAM_TOMBSTONED" }                => StreamTombstonedException.FromRpcException(ex),
					{ Reason: "APPEND_RECORD_SIZE_EXCEEDED" }      => AppendRecordSizeExceededException.FromRpcException(ex),

Evidence

The catch block throws the result of a switch expression whose default arm evaluates to the caught
ex; throwing an exception variable/expression (as opposed to throw;) resets the stack trace to
the current throw location, obscuring the original gRPC failure site.

src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs[81-89]
Best Practice: Microsoft Docs (.NET)

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MultiStreamAppendAsync` currently rethrows the caught `RpcException` using `throw <expression>` where the expression may evaluate to `ex`. This resets the stack trace to the catch block, reducing debuggability for transport-level gRPC failures.

## Issue Context
This PR’s goal is to avoid masking transport failures (null `GetRpcStatus()`) and surface the original `RpcException`. Preserving the original stack trace aligns with that goal.

## Fix Focus Areas
- src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs[81-89]

## Suggested approach
Rewrite the catch block to use statement-form control flow so the fallback path uses `throw;` (or `ExceptionDispatchInfo.Capture(ex).Throw()`) instead of `throw ex`:

- Compute `errorInfo = ex.GetRpcStatus()?.GetDetail<ErrorInfo>()`
- If `errorInfo?.Reason` matches a known reason, throw the typed exception
- Otherwise, `throw;` to rethrow the original exception with its stack trace preserved

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[Copilot]

Pull request overview

Fixes a crash path in the client’s gRPC error handling where RpcException.GetRpcStatus() can be null (transport-level failures), previously causing NullReferenceException and masking the original RpcException.

Changes:

• Update MultiStreamAppendAsync to use null-conditional access when reading ErrorInfo from GetRpcStatus().
• Update multiple FromRpcException factory methods to avoid null-forgiving GetRpcStatus()! and instead fall back to throwing the RpcException when no Google.Rpc.Status is present.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs	Avoid NRE when GetRpcStatus() is null during multi-append error mapping.
src/KurrentDB.Client/Streams/MaximumAppendSizeExceededException.cs	Guard against null GetRpcStatus() in exception factory method.
src/KurrentDB.Client/Core/Exceptions/WrongExpectedVersionException.cs	Guard against null GetRpcStatus() in exception factory method.
src/KurrentDB.Client/Core/Exceptions/StreamTombstonedException.cs	Guard against null GetRpcStatus() in exception factory method.
src/KurrentDB.Client/Core/Exceptions/NotLeaderException.cs	Guard against null GetRpcStatus() in exception factory method.
src/KurrentDB.Client/Core/Exceptions/AppendTransactionMaxSizeExceededException.cs	Guard against null GetRpcStatus() in exception factory method.
src/KurrentDB.Client/Core/Exceptions/AccessDeniedException.cs	Guard against null GetRpcStatus() in exception factory method.

Comments suppressed due to low confidence (1)

src/KurrentDB.Client/Streams/KurrentDBClient.MultiAppend.cs:88

• The default arm returns ex, which results in throw ex; and resets the original gRPC stack trace. Since this is inside a catch (RpcException ex), consider restructuring (e.g., assign mapped exception, and use throw; when no mapping applies) so transport-level failures truly rethrow the original exception without losing stack information.

					{ Reason: "STREAM_TOMBSTONED" }                => StreamTombstonedException.FromRpcException(ex),
					{ Reason: "APPEND_RECORD_SIZE_EXCEEDED" }      => AppendRecordSizeExceededException.FromRpcException(ex),
					{ Reason: "APPEND_TRANSACTION_SIZE_EXCEEDED" } => AppendTransactionMaxSizeExceededException.FromRpcException(ex),
					_                                              => ex
				};

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

@w1am 👉 Created pull request targeting release/v1.3: #406