Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Why doesn't RequestExceptionProcessorBehavior support null responses? #1094

Open
tonycoelho opened this issue Jan 20, 2025 · 3 comments

Comments

@tonycoelho
Copy link

RequestExceptionProcessorBehavior.Handle() doesn't support the ability to return a null response, i.e.:

if (!state.Handled)
{
    throw;
}

if (state.Response is null)
{
    throw;
}

return state.Response; //cannot be null if Handled

What is the reason for this? There are legitimate use cases where you might want to return a null response when an exception is handled.

@ScarletKuro
Copy link

ScarletKuro commented Jan 30, 2025

Nothing stops you from implementing your own RequestExceptionProcessorBehavior and replacing it in the DI. It's just a prebuilt exception preprocessor behavior, but the framework is quite flexible when it comes to replacing those parts.

@tonycoelho
Copy link
Author

Nothing stops you from implementing your own RequestExceptionActionProcessorBehavior and replacing it in the DI.

Yes, I know but the intent of the question is to understand from the authors what the purpose of the null check is in the default behavior.

@ScarletKuro
Copy link

ScarletKuro commented Jan 30, 2025

Yes, I know but the intent of the question is to understand from the authors what the purpose of the null check is in the default behavior.

I don’t think the owner will give you an answer that satisfies you. If I were the owner, I would just say it’s historical reasons.

The Exception Handler Behaviors were a community pull request, and it didn’t have any null checks or throw an exception on null values: #339. Later, the community added nullable support: #518, and it was decided that TResponse shouldn’t be null:

return state.Response!; //cannot be null if Handled

but this still wouldn’t throw an exception if null was set in the response.

Then, during the C# 9 update, this code was refactored. The person working on it saw that "cannot be null" removed the ! workaround, and added this code:

if (!state.Handled)
{
throw;
}
return state.Response!; //cannot be null if Handled

However, RequestExceptionHandlerState has Response marked as potentially null:
public TResponse? Response { get; private set; }

Which means setting Response to null is a valid value according to the annotation. Therefore, the exception pipeline probably shouldn't throw an exception in this case.

As I see it, this seems like a coincidence: it wasn’t checking for null initially, but during the annotation process, it was concluded that the value "cannot" be null, and later, it was interpreted as something that should throw an exception.

What I would suggest is that you fix the issue, create a test, and submit a pull request describing your use case and why it’s useful. I’m sure it will be accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants