logging-log4net-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Johnson, Thomas" <Thomas.John...@lpsvcs.com>
Subject RE: Proposed bug fix for AspNetRequestPatternConverter.Convert()
Date Fri, 30 Mar 2012 21:54:18 GMT
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:george@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.mlist@phaseit.com<mailto:box.mlist@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 <george@glympse.com<mailto:george@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.Johnson@lpsvcs.com<mailto:Thomas.Johnson@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<mailto: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:george@glympse.com<mailto:george@glympse.com>]
Sent: Friday, March 30, 2012 1:29 PM
To: log4net-dev@logging.apache.org<mailto: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<http://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<http://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<http://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.

Mime
View raw message