search

NickCraver / StackExchange.Exceptional

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

Late initialisation of ExceptionalModule resets configuration #169

Closed MattJeanes closed 4 years ago

MattJeanes commented 5 years ago

This was a super difficult one to track down. I was having really weird behaviour, errors would log fine to the SQL error log but the moment I tried to observe the errors page (using return ExceptionalModule.HandleRequestAsync(System.Web.HttpContext.Current); it would switch to the memory log.

Turns out this is caused by late initialisation of the static class ExceptionModule which can happen if you are not using the ExceptionModule in the web.config as a module. For example, I have abstracted away Exceptional into our own LoggerModule which in turns logs to Exceptional (and serilog).

Init of ExceptionModule calls the internal method StackExchange.Exceptional.Settings.LoadSettings() which just flat out replaces the configuration settings with data from the web.config and disregards any previously set-up code-based config.

A potential fix could be to try and merge the configuration rather than simply replacing it, but I wasn't sure what you'd want to do here so I haven't written a PR for it.

I'm working around the issue by simply inheriting my LoggerModule from ExceptionalModule and overriding all of its methods (thx for making them virtual!!) so that it still calls the LoadSettings on startup before my code-based config gets set.

NickCraver commented 4 years ago

I think what you've done is the workaround I'd suggest going with here. The alternative is breaking and it's safest to assume current order (since later settings may be derivative and unmerge-able). Unfortunately if we don't do it in the static initializer (as was in previous versions), there are just so many cases that result in null settings - we must ensure they're loaded.

Another alternative is to simple touch ExceptionalModule early (though not used) to trigger its initializer. It's not the cleanest thing in the world, but is a simpler alternative.

MattJeanes commented 4 years ago

Yeah I can definitely appreciate not making breaking changes. I ended up accidentally doing the 'touch' workaround accidentally while debugging the problem which is how I worked out what was happening, that was several hours of fun! No worries, hopefully if someone else runs into this they'll find this issue ☺