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

Unnecessary creation of an error object which is never used? #4465

Open
StefanOssendorf opened this issue Jan 30, 2025 · 3 comments
Open

Unnecessary creation of an error object which is never used? #4465

StefanOssendorf opened this issue Jan 30, 2025 · 3 comments

Comments

@StefanOssendorf
Copy link
Contributor

StefanOssendorf commented Jan 30, 2025

During my NRT work I've found the referenced code.

catch (Exception ex)
{
result.ErrorData = _applicationContext.CreateInstance<DataPortalErrorInfo>(_applicationContext, ex);
throw;
}
finally
{
result = ConvertResponse(result);
}

My understanding is that we create the DataPortelErrorInfo and assign it to the ErrorData property. But the result object is never actually returned. The only "value" is the ConverResponse picks up the error but can't do a thing to influence the program flow.
Should we remove the unnecessary object creation?
It affects the following portals:

  • HttpPortal
  • GrpcPortal
  • RabbitMqPortal
@rockfordlhotka
Copy link
Member

I see, the throw means the result is never returned, and the actual ErrorData property must be set at a higher level in the workflow on the server.

So yes, this is probably a useless catch due to some change that occurred over time and left this code behind.

@StefanOssendorf
Copy link
Contributor Author

Or the throw has to be removed. I'm not sure if the finally block is in a case of an exception obsolete, too?
A user can "ConvertResponse" to something that's entirely ignored.

@rockfordlhotka
Copy link
Member

When this is addressed, whoever does the work needs to look at the higher level and see how (and if) the error info is created, and ensure that the ConvertResponse is invoked in some manner. That's the hook for compression or encryption or whatever someone would want to do to the byte stream.

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

No branches or pull requests

2 participants