NickCraver / StackExchange.Exceptional

Error handler used for the Stack Exchange network
https://nickcraver.com/StackExchange.Exceptional/
Apache License 2.0
863 stars 171 forks source link

Added support for paging Error List #59

Closed dirkoswanepoel closed 9 years ago

NickCraver commented 9 years ago

Hey @dirkoswanepoel, Thanks for the request.

I have consciously left out paging for several reasons here - namely our use case for that is Opserver which has much richer support. Specifically with this request there are issues as well:

  1. In SQL, you're trusting the JSON (and have the cost of deserializing it) for the errors. This is incorrect. For instance the DuplicateCount in SQL comes from the table column, not the JSON. The same is true for checking if an error has been dealt with and self deleted. Basically this would always result in a list of not-deleted single-count errors.
  2. The SQL behavior doesn't match any other methods, because of #1, introducing unexpected behavior.
  3. The JSON Error store is no longer DRY-coded, the method is mostly duplicated
  4. The pull request itself is mostly white-space changes (please don't do this to anyone!).

Now this answer/response is going to suck: I do not want paging in Exceptional. It encourages not dealing with exceptions in the log and just letting them rot. The current model of deletion and protection are design to be the mechanism for dealing with a lot of errors here (since dealing with and deleting an error removes it from the list). I believe this works well because we use it for Stack Overflow and many other services, even when we can throw billions of exceptions an hour in extreme cases. For the very common non-SQL use case (primarily JSON and Memory) paging means an extreme performance hit if expanding the overall around stored. I also have a large C# 6 changeset for Exceptional to push next week, but that isn't my reason for declining this - it's a matter of code behavior and encouraging proper developer behavior.