scientistproject / Scientist.net

A .NET library for carefully refactoring critical paths. It's a port of GitHub's Ruby Scientist library
MIT License
1.46k stars 95 forks source link

Option to re-throw original exception instead of inner exception? #99

Closed oxfordemma closed 6 years ago

oxfordemma commented 7 years ago

Observation.cs#L126

We're losing a lot of information when this re-throws the inner exception rather than the original exception.

Open to always using the original exception or adding an option somewhere to use the original exception instead?

davezych commented 7 years ago

In what scenarios are you losing information? I can't recall the exact reason, but I think this was set up to always throw the inner exception due to the way exceptions get thrown/caught in async methods.

oxfordemma commented 7 years ago

I'm using the async method Scientist.ScienceAsync<T>(...).

In my case I'm getting an AmazonDynamoDBException which has an inner exception of HttpErrorResponseException.

The inner exception is useless, but that's what gets returned by ex.GetBaseException() and re-thrown.

I think you are thinking of AggregateException. If mine was wrapped in that, then AggregateException.GetBaseException() would return the correct thing. Otherwise, it keeps following InnerException until there's not one to follow anymore.

Async/await only wraps the exception with AggregateException if you use the blocking methods Wait()/Result. If I switch to those, then it works as expected.

What about something like:

try
{
    Value = await block();
}
catch (AggregateException ex)
{
    Exception = ex.GetBaseException();
}
catch (Exception ex)
{
    Exception = ex;
}
davezych commented 7 years ago

Ahh yes, that's what I was thinking.

The above proposal looks good to me - make it happen!