NickCraver / StackExchange.Exceptional

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

The modules section in web.config is always required #63

Closed cottsak closed 6 years ago

cottsak commented 8 years ago

The wiki page https://github.com/NickCraver/StackExchange.Exceptional/wiki/Setup under the section This is all optional, you can setup completely via code as well. Examples suggests that all of the web.config setup can be skipped if you just use the ErrorStore.Setup() method.

I found however that I still need at minimum the

  <system.webServer>
    <modules>
      <add name="ErrorStore" type="StackExchange.Exceptional.ExceptionalModule, StackExchange.Exceptional" />
    </modules>
  </system.webServer>

to catch the exceptions.

Can we somehow make this requirement clearer? It will save a lot of folks a lot of time I think.

CptRobby commented 8 years ago

Actually, I don't think the ExceptionalModule is required either. You could (for example) define an Application_Error function in your global.asax handler and call the ErrorStore.LogException method from there. There might be some minor differences between how these two methods of logging unhandled exceptions would behave (IDK), but I'd still say that labeling the ExceptionalModule a minimal requirement is a bit of a stretch - especially since it wouldn't be usable at all in a non-ASP.NET project.

But I do agree that the documentation is quite lacking on this project.

cottsak commented 8 years ago

@CptRobby Haven't tested your suggestion but if it does work, then at least we can agree that one of the two alternatives is required as a minimum. All I'm saying is that the doco should get someone up and running with minimum fuss and as it reads now, I'm going to have a 'hole' with the 'code only' (ie, no web.config) version.

CptRobby commented 8 years ago

@cottsak Not trying to be nit-picky, but I personally wouldn't call either option a minimal requirement. Your complaint isn't really that it does't highlight something as being required, but with a wiki page from 3 years ago that gives an example of how someone could use Exceptional and merely mentions that it's also possible to use it without putting stuff in web.config without really explaining it further. Don't get me wrong, I'm completely in agreement that there needs to be better documentation (see #53). I'm just saying that it would be incorrect to say something like "In order to use Exceptional in your project, you MUST include this section in your web.config or this block of code in global.asax." That would give the impression that Exceptional couldn't be used for desktop applications or that there's some kind of magic that happens when you do those things. The only important thing that happens when you do either of those things is a call to ErrorStore.LogException, which is exactly what the wiki says is needed for logging exceptions. And there are plenty of other places where you could call that method in response to an exception. What's really needed though is some real documentation, not just an update to a quick start wiki page.

So are you really needing to get this working without web.config? Are you running into any other issues? I'd be glad to help out if you need any. :smile:

cottsak commented 8 years ago

Here is a simple suggestion then: https://github.com/NickCraver/StackExchange.Exceptional/wiki/Setup/7f97562a644182204b9d9350f3ca7dd940cdb17f

nickalbrecht commented 8 years ago

On a somewhat related note of implementation via code instead of config file; I went digging through the code but I could find no information to indicate if the LogFilters are configurable via code. Is the config section the only option for the filters?

CptRobby commented 8 years ago

@nickalbrecht Look into the Error.FormLogFilters and Error.CookieLogFilters collections. If you check Error.cs, you'll see that the LogFilters from the config settings are just copied into those collections in the static Error constructor and then used in Error.SetContextProperties, which is called from the regular Error constructor.

Bottom line is that you could replace this:

<LogFilters>
  <Form>
    <add name="password" replaceWith="********" />
  </Form>
  <Cookies>
    <add name="authToken" replaceWith="********" />
  </Cookies>
</LogFilters>

with this:

StackExchange.Exceptional.Error.FormLogFilters["password"] = "********";
StackExchange.Exceptional.Error.CookieLogFilters["authToken"] = "********";

which would be called in your global.asax Application_Start method, or whatever startup event your application would use.

Hope that helps! ;)

IsaackRasmussen commented 8 years ago

I really agree with @cottsak - I spend alot of time trying to figure out what was wrong and adding that line is what made it work. And this is both for catching unhandled and even ErrorStore.LogException().

@CptRobby I even tried your global.asax suggestion and its not logging from there either without the line.

So what is the alternative, if that line really isn't required... for now, I'm just gonna add it to my websites. Looking forward to when it comes to my services, if they will work without :)

CptRobby commented 8 years ago

@IsaackRasmussen I'm pretty sure it should have worked in about the same way. What was the code that you tried in global.asax? Did you look at the source code for ExceptionalModule (StackExchange.Exceptional/StackExchange.Exceptional/ExceptionalModule.cs) to see what it actually does? Is your ErrorStore configured in web.config or in code? If it's configured in code, where is that code located? Just trying to help find the answer to your question. :wink:

For my websites, I just add the ExceptionalModule to the web.config since it doesn't involve writing additional code and the ErrorStore is also configured there. I've not noticed any downside to doing it that way, but I'm curious if there are other ASP.NET situations where that isn't an option. What type of services are you planning on adding that you are wondering about?

IsaackRasmussen commented 8 years ago

@CptRobby its not a problem for me at all to use web.config. I just decided to do it code only.

I set it up with SQL Store from the example StackExchange.Exceptional.ErrorStore.Setup("My Application", new SQLErrorStore(_connectionString));

And then: ErrorStore.LogException(exception, _context); (Also tried without context)

And I placed it in protected void Application_Start()

I had looked at some of the source files but not that one. And now that I look at it, its not really helping.

CptRobby commented 8 years ago

@IsaackRasmussen Just to clarify, you put the ErrorStore.Setup line in Application_Start, and the ErrorStore.LogException line in Application_Error, right?

The code for the ExceptionalModule is pretty simple. It's an IHttpModule, which gets loaded when your web application is started up. When the Init function is called and given a reference to the HttpApplication, it added an event handler for the HttpApplication.Error event (which is triggered when an unhandled exception occurs). In the OnError function that handles that event, it calls the HttpApplication.Server.GetLastError method to grab the unhandled exception and then sends it on to ErrorStore.LogException.

Conceptually, global.asax defines your HttpApplication, so the Application_Error method should behave about the same as attaching a handler to the HttpApplication.Error event.

Here's a funny story for you though. When I first started using Exceptional, I followed the tutorial and just added the ExceptionalModule to my web.config without knowing what it actually did. I then went through my code adding ErrorStore.LogException to all my catch blocks that I wanted to record. But I knew that I also wanted to log any unhandled exceptions, so I added ErrorStore.LogException to my global.asax Application_Error method. It took me a good bit of searching and debugging to figure out why some errors were getting logged twice! :laughing:

IsaackRasmussen commented 8 years ago

@CptRobby

Actually, I invoked LogException from both Application_start and one of my controllers, just to test with a made up exception. This did not work until I added the line in Web.config. I can't see why it would change adding it to Application_Error, as it wasn't working two other places.

And regarding your funny story, hehe, I was just about to make the same mistake but luckily realized in time that it was already catching unhandled exceptions.

CptRobby commented 8 years ago

So when you say you tried it out using a made up exception, do you mean that you did something like

ErrorStore.LogException(new Exception("Testing"), Context);

or was it more like

try
{
    throw new Exception("Testing");
}
catch (Exception ex)
{
    ErrorStore.LogException(ex, Context);
}

If it was the first one, I doubt that would ever work correctly since the exception hasn't been thrown yet.

As for Application_Error vs Application_Start, I would strongly suggest using Application_Error since that's what it's for. It should look something like this:

protected void Application_Error()
{
    ErrorStore.LogException(Server.GetLastError(), Context);
}

You should then be able to set a breakpoint there and just throw an exception from a controller (without catching it) and you should see it make the call and log the exception.

Of course if you're settled on just using web.config, I understand if you don't want to go to the trouble of changing everything around. :smile:

IsaackRasmussen commented 8 years ago

@CptRobby yes, it was the first one. I had the idea that you could always call LogException to log any kind of Exception, whether thrown or not. But then again, when I add the modules line in web.config... the first example does work.

I'll Application_Error now and remove the module line, just to prove myself wrong. I still don't think it will work :)

IsaackRasmussen commented 8 years ago

@CptRobby thanks for the patience! It does actually work from application_error without the web.config module line. I did the testing with array overflows and null exceptions.

However, when I have the web.config line. Then I can do this anywhere in my code ErrorStore.LogException(new Exception("Testing"), Context);

I changed to application_error anyway, as I like being able to set one global breakpoint when debugging.

CptRobby commented 8 years ago

@IsaackRasmussen Well it's in the throwing of an exception that things like the stack trace and everything else gets populated. But I did just try it out and it does work on my system (using the ExceptionalModule in web.config) though it only logged the Exception.Message and not much else. No idea why it would behave differently because of that though. :confused:

You're very welcome though and I'm really glad that you were able to get it working the way you wanted it to. :+1: :smile:

sutharmonil commented 7 years ago

@CptRobby This actually works:

static void Main(string[] args)
{
    var dict = new Dictionary<string, string>();
    dict.Add("key", "value");
    ErrorStore.LogExceptionWithoutContext(new Exception("Testing1"), true, false, dict);
    ErrorStore.LogExceptionWithoutContext(new Exception("Testing101"), false, false, dict);
    //SQLErrorStore.LogExceptionWithoutContext(new Exception("Testing"), true, false, dict);
}
NickCraver commented 6 years ago

Resolved in v2, so closing this out :)