davybrion / Agatha

Agatha Request-Response Service Layer for .NET
http://davybrion.github.com/Agatha/
Other
86 stars 39 forks source link

Common Logging #22

Open mdmoura opened 12 years ago

mdmoura commented 12 years ago

Hello,

I would like to report an issue with Common Logging in Agatha:

The latest change in RequestProcessor.cs was incomplete. Now each error is logged twice:

1 - First one with "RequestProcessor: unhandled exception while handling request!" as message;

2 - Second one with exception message as message.

I think the same change must be done in RequestProcessor LINE 260:

protected virtual void OnHandlerException(OneWayRequest request, Exception exception) {
  logger.Error("RequestProcessor: unhandled exception while handling request!", exception);
}

Finally, I have a suggestion:

On RequestIProcessor.cs, LINE 12, there is the following:

private readonly ILog logger = LogManager.GetLogger(typeof(RequestProcessor));

Could this be changed to something like:

private readonly ILog logger = LogManager.GetLogger(loggerName));  

Where loggerName would be defined on Agatha Setup.

If it is not defined then it would get the default value: typeof(RequestProcessor)

Thank You, Miguel

davybrion commented 12 years ago

the problem was that the exceptions were logged both in the virtual OnHandlerException methods, as well as in the code that's around the calling of the request handlers.

i think the correct solution is to make those virtual OnHandlerException methods no-ops, because we need the other log statements in the other catch blocks because they catch more than just handler exceptions. I'm gonna commit it like that for now.

mdmoura commented 12 years ago

And could you add an option to define the loggerName? At the moment is always "Agatha.ServiceLayer.RequestProcessor" due to "typeof(RequestProcessor)"

There could be something like (LoggerName to be added at the end as an option):

new ServiceLayerAndClientConfiguration(typeof(CreateUserHandler).Assembly, typeof(CreateUserRequest).Assembly, new Agatha.StructureMap.Container(ObjectFactory.Container), "MyLoggerName").Initialize();

Thanks, Miguel

mdmoura commented 12 years ago

"i think the correct solution is to make those virtual OnHandlerException methods no-ops, because we need the other log statements in the other catch blocks because they catch more than just handler exceptions. I'm gonna commit it like that for now."

You mean some will log the exception.Message and others will log "RequestProcessor: unhandled exception while handling request!"?