Capture additional exception details in the test output

[drewburlingame]

When our applications throw exceptions, we capture additional context in the .Data property of the exceptions. We would like to include this context in the NUnit output to reduce the time spent combing through logs and stepping through the debugger.

I've created two options for capturing additional information from exceptions. https://github.com/drewburlingame/nunit/pull/1

1. Capture the exception in a new ExceptionResult.Exception property.
   Pros:
   * A straightforward change.
   * It can be inspected by components for different purposes.
   Con:
   * Including the exception in the output of tests is more complicated, requiring an additional wrapper. The output will not be included as part of the default NUnit error message

2. . Register a static delegate with ExceptionHelper.GetAdditionalDetails
   Pros:
   * A straightforward change.
   * Very simple to use
   * Context is included with the exception message, making it easier to correlate.
   Cons:
   * Doesn't fit with NUnit pattern of providing extensibility points via interfaces
   * Global to the app domain. Cannot be enabled only for select fixtures. This isn't an issue for our use case but I assume there are cases where this may be an issue.

Would either of these be acceptable solutions? If not, do you have recommendations for how you'd like to see it?

[CharliePoole]

While I can see doing this as an extensibiliy mechanism, use of the Data property might be common enough to make it a standard feature. That leads me to favor the first option.

Of course, that begs the question of how and when we will publicly expose what we find there. I'd say definitely in the XML output but possibly not in the standard message. What do others think?

[drewburlingame]

I think the first option makes sense because it gives more flexibility. It also enables accessing properties on the exceptions.

Including custom and Data properties in XML output would provide useful information.

I use VS w/ R# and Rider and only the standard message is shown. This wouldn't have much value for me unless we could include it in the standard message. I would be ok with configuring the behavior in a setup method if perf or backward compatibility is a concern.

[CharliePoole]

What do other @nunit/framework-team members think? Is the actual text format of the message we display for an unexpected exception a compatibility issue? I would lean toward saying that the format has never been standardized and varies from platform to platform, so it can be whatever we want.

Edit: And it's also language-specific.

[rprouse]

I think that the message format isn't a compatibility issue as long as it remains clear and readable. We've improved messages in the past to make the errors easier to understand and I don't think this is very different.

I'm not sure it should be a standard feature. If we did that, which other properties on Exception would we include? How would we format the message with inner exceptions or aggregate exceptions? I think an extensibility point would be good for this. We don't do all extensibility via interfaces, so I wouldn't rule out option 2 because of that. The global problem could be solved by adding something to the TestContext, but then you would have to set it up everywhere.

My thinking is that if you want to change the formatting of the messages, you would want to do it at the assembly level for the entire test suite. Do you disagree?

[CharliePoole]

My thought was to simply do it automatically in the case where the user had added entries to the Data dictionary. Some .NET exceptions may add to this dictionary as well, so we might need to filter out those entries. Ideally, the message would only be changed if the user had added something. I think anything beyond the Data dictionary would have to be handled as an extension.

[drewburlingame]

In addition to the Data dictionary, properties not defined on the Exception base class could be included. I've found it heplful with SqlExceptions.

[drewburlingame]

Should I post a few PRs for this? I'm thinking

1. Capture the exception in a new ExceptionResult.Exception property
2. update existing behavior to output Data dictionary (and maybe properties not defined on Exception base class)?
3. Register a static delegate with ExceptionHelper.GetAdditionalDetails - could add an example that loops through inner exceptions

[CharliePoole]

Maybe two stages? First handle data dictionary?

[stevenaw]

I like the idea of including Exception.Data a lot.
Sometimes in the past I've used it to capture any manner of context at the site of an exception which may not make sense to embed in a message itself (ex: for application logging reasons)

[jnm2]

I just hit this feature for the first time in a real world project without expecting it and I really love it!