Yes that's exactly the point. If log4net need the request and you're calling it in a scope that doesn't have it, catching the error in log4net doesn't seem a viable solution for my point of view because it'll not work as expected without request.
Probably it's better to move the app initializiation code in a scope that actually have the request available as you can read in the article. Best Alessandro Ghizzardi Moving... Il giorno 30/mar/2012, alle ore 23:54, "Johnson, Thomas" <thomas.john...@lpsvcs.com> ha scritto: > It sounds like he’s suggesting you fix your site instead of the codebase? If > there’s definitely a fix that needs to be in place, I would explore the > httpContext to see if you can infer whether get_Request would throw an NRE > and check that instead of try/catch – it’s just a code smell everyone should > try to avoid in general > > From: George Chung [mailto:geo...@glympse.com] > Sent: Friday, March 30, 2012 3:50 PM > To: Log4NET Dev > Subject: Re: Proposed bug fix for AspNetRequestPatternConverter.Convert() > > Yes, that is exactly the issue. Just my humble opinion, but Log4Net would > only incur the try/catch perf penalty during user logging inside > Application_Start, which is a very tiny percentage of the lifetime of an > ASP.Net application. > > The issue for log4net is that people staticly define their layout patterns > that they want in the web.config. It's not obvious to me how they could > "instruct" log4net to ignore the aspnet layout patterns while inside > Application_Start. > > I don't know any other way to test the viability of calling through > get_Request() before the fact. > > Cheers, > George > > On Fri, Mar 30, 2012 at 2:30 PM, Alessandro Ghizzardi <box.ml...@phaseit.com> > wrote: > Hi George, > > It seems more like an architecture problem in logging that point, instead of > a bug. It used to work in older version, but probably you're using IIS7 in > integrated mode and request is not available anymore. > If you really need to log in application start probably this workaround can > help you > > http://mvolo.com/blogs/serverside/archive/2007/11/10/Integrated-mode-Request-is-not-available-in-this-context-in-Application_5F00_Start.aspx > > Best > Alessandro Ghizzardi > Moving... > > Il giorno 30/mar/2012, alle ore 22:59, George Chung <geo...@glympse.com> ha > scritto: > > The subtlety is that in Application_Startup(), httpContext is indeed not null > and httpContext.Request is really httpContext.get_Request(). > > And it is the act of calling get_Request() that throws the HttpException. > > On Fri, Mar 30, 2012 at 1:51 PM, Johnson, Thomas <thomas.john...@lpsvcs.com> > wrote: > Sorry: if (httpContext != null && httpContext.Request != null) > > Since you’re in c# the second statement won’t be evaluated if the first is > false and you will avoid NRE > > From: Johnson, Thomas > Sent: Friday, March 30, 2012 2:50 PM > To: log4net-dev@logging.apache.org > Subject: RE: Proposed bug fix for AspNetRequestPatternConverter.Convert() > > Typically you want to avoid code like this // maybe just do: if (httpContext > != null) { //the entirety of your method } else > writer.Write(SystemInfo.NotAvailableText); > > try > { > request = httpContext.Request; > if (request == null) > > requestExists = false; > } > catch (System.Web.HttpException) > > { > requestExists = false; > } > > > From: George Chung [mailto:geo...@glympse.com] > Sent: Friday, March 30, 2012 1:29 PM > To: log4net-dev@logging.apache.org > Subject: Proposed bug fix for AspNetRequestPatternConverter.Convert() > > Hello, > I'm new to this mailing list. Sorry if this is not the appropriate way to > report a bug fix. Dereferencing HttpContext.Request will throw an exception > if you call it in an Asp.net application's Application_Start(). > > Thus, if you have log4net logging *and* you use a pattern layout format like > %aspnet-request{REMOTE_ADDR} *and* you call Log4net inside > Application_Start(), Log4net will throw an exception trying to log the > message. The stack will look like this: > > log4net:ERROR [RollingFileAppender] ErrorCode: GenericFailure. Failed in > DoAppend > System.Web.HttpException (0x80004005): Request is not available in this > context > at System.Web.HttpContext.get_Request() > at log4net.Layout.Pattern.AspNetRequestPatternConverter.Convert(TextWriter > writer, LoggingEvent loggingEvent, HttpContext httpContext) > at log4net.Layout.Pattern.AspNetPatternLayoutConverter.Convert(TextWriter > writer, LoggingEvent loggingEvent) > at log4net.Layout.Pattern.PatternLayoutConverter.Convert(TextWriter > writer, Object state) > at log4net.Util.PatternConverter.Format(TextWriter writer, Object state) > at log4net.Layout.PatternLayout.Format(TextWriter writer, LoggingEvent > loggingEvent) > at log4net.Layout.Layout2RawLayoutAdapter.Format(LoggingEvent loggingEvent) > > > Here is my proposed fix: > > > internal sealed class AspNetRequestPatternConverter : > AspNetPatternLayoutConverter > { > /// <summary> > /// Write the ASP.Net Cache item to the output > /// </summary> > > /// <param name="writer"><see cref="TextWriter" /> that will receive > the formatted result.</param> > /// <param name="loggingEvent">The <see cref="LoggingEvent" /> on > which the pattern converter should be executed.</param> > /// <param name="httpContext">The <see cref="HttpContext" /> under > which the ASP.Net request is running.</param> > /// <remarks> > > /// <para> > /// Writes out the value of a named property. The property name > /// should be set in the <see > cref="log4net.Util.PatternConverter.Option"/> > /// property. > /// </para> > /// </remarks> > > protected override void Convert(TextWriter writer, LoggingEvent > loggingEvent, HttpContext httpContext) > { > bool requestExists = true; > > HttpRequest request = null; > try > { > request = httpContext.Request; > if (request == null) > requestExists = false; > } > catch (System.Web.HttpException) > > { > requestExists = false; > } > > > if (requestExists) > { > if (Option != null) > > { > WriteObject(writer, loggingEvent.Repository, > request.Params[Option]); > } > else > { > WriteObject(writer, loggingEvent.Repository, > request.Params); > } > } > > else > { > > writer.Write(SystemInfo.NotAvailableText); > } > } > } > The information contained in this message is proprietary and/or confidential. > If you are not the intended recipient, please: (i) delete the message and all > copies; (ii) do not disclose, distribute or use the message in any manner; > and (iii) notify the sender immediately. In addition, please be aware that > any message addressed to our domain is subject to archiving and review by > persons other than the intended recipient. Thank you. > > > The information contained in this message is proprietary and/or confidential. > If you are not the intended recipient, please: (i) delete the message and all > copies; (ii) do not disclose, distribute or use the message in any manner; > and (iii) notify the sender immediately. In addition, please be aware that > any message addressed to our domain is subject to archiving and review by > persons other than the intended recipient. Thank you.
