Bug 21796

Summary: SocketAppender doesn't fall back with FallbackErrorHandler
Product: Log4j - Now in Jira Reporter: Mirek Rusin <miroslaw.rusin>
Component: AppenderAssignee: log4j-dev <log4j-dev>
Status: RESOLVED FIXED    
Severity: normal CC: zano
Priority: P3    
Version: 1.2   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Cleaned up patch file of the pasted text in this issue

Description Mirek Rusin 2003-07-22 13:32:34 UTC
...i was playing with FallbackErrorHandler and came up
against problem with SocketAppender:

i set up logger
 with primary appender (SocketAppender)
  with errorHandler (FallbackErrorHandler)
   with backup appender

SocketAppender has ReconnectionDelay = 0 (no reconnecting)

problem occurs when SocketAppender meets connection-error:

1. when connection establishment fails
2. when connection is interrupted

...and ReconnectionDelay equals 0, SocketAppender is down forever.
in both cases errorHandler's error method invokation is missing
(in SocketAppender.java) when reconnectionDelay is 0.

here's org/apache/log4j/net/SocketAppender.java fix-diff

####### diff #######
56a57
> import org.apache.log4j.spi.ErrorCode;
259a261,264
>       else {
>         msg += " We are not retrying.";
>         errorHandler.error(msg, e, ErrorCode.GENERIC_FAILURE);
>       }
311a317,320
>         else {
>           errorHandler.error("Detected problem with connection, not 
reconnecting.", e,
>           ErrorCode.GENERIC_FAILURE);
>         } 
####### diff #######



test case:

note1: exception is ok, it just says that connection failed, that's
what we want. error handler should fallback and replace primary
appender with backup appender

note2: it's just test for '1. when connection establishment fails' case


####### SocketAppenderTest.java #######

import java.net.*;
import java.io.*;

import junit.framework.*;

import org.apache.log4j.*;
import org.apache.log4j.net.*;
import org.apache.log4j.spi.*;
import org.apache.log4j.xml.*;
import org.apache.log4j.helpers.*;


public class SocketAppenderTest extends TestCase
{
        
        public static void main(String args[])
        {
                junit.textui.TestRunner.run(SocketAppenderTest.class);
        }
        

        /* JUnit's setUp and tearDown */
        
        protected void setUp()
        {
                DOMConfigurator.configure("SocketAppenderTestLog4j.xml");

                logger = Logger.getLogger(SocketAppenderTest.class);
                primary = logger.getAppender("remote");
                secondary = (LastOnlyAppender)Logger.getLogger
("SocketAppenderTestDummy").getAppender("lastOnly");
        }

        protected void tearDown()
        {
                // null-save interrupt
                if (serverThread != null && !serverThread.interrupted())
                        serverThread.interrupt();
        }

        
        /* Tests */
        
        public void testFallbackErrorHandlerWhenStarting()
        {
                String msg = "testFallbackErrorHandlerWhenStarting";
                logger.debug(msg);

                // above debug log will fail and shoul be redirected to 
secondary appender
                assertEquals("SocketAppender with FallbackErrorHandler", msg, 
secondary.getLastMessage());
        }
        
        
        /* Helper methods */
        /* INFO: it's unused
        private void startServerThread()
        {
                // null-save interrupt
                if (serverThread != null && !serverThread.interrupted())
                        serverThread.interrupt();
                
                serverThread = new Thread()
                        {
                                public void run()
                                {
                                        try
                                        {
                                                int port = 
8189; //primary.getPort();
                                                ServerSocket serverSocket = new 
ServerSocket(port);
                                                Socket socket = 
serverSocket.accept();
                                                Hierarchy h = new
Hierarchy(Logger.getLogger("SocketAppenderTestServerThread"));
                                                new SocketNode(socket,h).run();
                                        }
                                        catch (IOException e)
                                        {
                                                System.err.println("server 
thread io exception");
                                        }
                                }
                        };
                serverThread.start();
        }*/
        

        /* Fields */
        
        private static Logger logger;
        private static Appender primary;
        private static LastOnlyAppender secondary;
        private static Thread serverThread;
        /* Inner classes */
        
        /** Inner-class
         *  For debugging purposes only
         *  Saves last LoggerEvent
         */
        static public class LastOnlyAppender extends AppenderSkeleton
        {
                protected void append(LoggingEvent event)
                {
                        this.lastEvent = event;
                }

                public boolean requiresLayout()
                {
                        return false;
                }

                public void close()
                {
                        this.closed = true;
                }
                
                /** @return last appended LoggingEvent's message
                 */
                public String getLastMessage()
                {
                        if (this.lastEvent != null)
                                return this.lastEvent.getMessage().toString();
                        else
                                return "";
                }

                private LoggingEvent lastEvent;
        };
        
}
####### /SocketAppenderTest.java #######


####### SocketAppenderTestLog4j.xml #######
<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">

<log4j:configuration debug="true" 
xmlns:log4j='http://jakarta.apache.org/log4j/'>


        <!-- primary appender -->
        <appender name="remote" class="org.apache.log4j.net.SocketAppender">

                <!-- fallback on error -->
                <errorHandler 
class="org.apache.log4j.varia.FallbackErrorHandler">
                        <logger-ref ref="SocketAppenderTest"/>
                        <appender-ref ref="lastOnly"/>
                </errorHandler>
                
                <param name="RemoteHost" value="localhost"/>
                <param name="Port" value="8189"/>
                <param name="ReconnectionDelay" value="0"/>
        </appender>

        
        <!-- secondary appender -->
        <appender name="lastOnly" 
class="SocketAppenderTest$LastOnlyAppender"></appender>

        
        <!-- logger -->
        <logger name="SocketAppenderTest">
                <appender-ref ref="remote"/>
        </logger>

        
        <!-- logger
             dummy, so i can have early access to lastOnly appender instance
        -->
        <logger name="SocketAppenderTestDummy">
                <appender-ref ref="lastOnly"/>
        </logger>

        
</log4j:configuration>

####### /SocketAppenderTestLog4j.xml #######
Comment 1 Yoav Shapira 2004-12-13 17:57:48 UTC
Can you please attached your patch (in diff -u format) to this Bugzilla issue, 
instead of embedding it here?  Thank you.
Comment 2 Yoav Shapira 2004-12-15 16:33:23 UTC
*** Bug 25475 has been marked as a duplicate of this bug. ***
Comment 3 Paul Smith 2007-04-24 22:46:31 UTC
Created attachment 20034 [details]
Cleaned up patch file of the pasted text in this issue

I took the text of the patch pasted into this issue, cleaned it up, and applied
it to a local logj 1.2 working copy.  the test does fail, and after working out
where the actual code change patch was supposed to go, it does pass.

Should I apply this patch to the 1.2 branch?
Comment 4 Curt Arnold 2007-04-26 12:47:32 UTC
Committed in rev 532842.