Merge pull request #4856 from jakewins/2.3-dontlogstacktracethrice

Only log fatal stack trace once in user log
neo4j · Jul 1, 2015 · 37d90c9 · 37d90c9
2 parents f006251 + 345623f
commit 37d90c9
Show file tree

Hide file tree

Showing 10 changed files with 31 additions and 35 deletions.
diff --git a/.../ndp/messaging-v1/src/main/java/org/neo4j/ndp/messaging/v1/PackStreamMessageFormatV1.java b/.../ndp/messaging-v1/src/main/java/org/neo4j/ndp/messaging/v1/PackStreamMessageFormatV1.java
@@ -395,7 +395,7 @@ else if ( obj instanceof Path )
             }
             else
             {
-                throw new RuntimeException( "Unpackable value " + obj + " of type " + obj.getClass().getName() );
+                throw new NDPIOException( Status.General.UnknownFailure, "Unpackable value " + obj + " of type " + obj.getClass().getName() );
             }
         }
     }
@@ -673,11 +673,11 @@ private Object unpackValue() throws IOException
                     return new ValuePath( entities );
                 }
                 default:
-                    throw new IOException( "Unknown struct type: " + signature );
+                    throw new NDPIOException( Status.Request.InvalidFormat, "Unknown struct type: " + signature );
                 }
             }
             default:
-                throw new IOException( "Unknown value type: " + valType );
+                throw new NDPIOException( Status.Request.InvalidFormat, "Unknown value type: " + valType );
             }
         }
 

diff --git a/community/ndp/runtime/src/main/java/org/neo4j/ndp/runtime/internal/ErrorTranslator.java b/community/ndp/runtime/src/main/java/org/neo4j/ndp/runtime/internal/ErrorTranslator.java
@@ -39,17 +39,15 @@
  */
 public class ErrorTranslator
 {
-    private final Log internalLog;
     private final Log userLog;
 
     public ErrorTranslator( LogService logging )
     {
-        this(logging.getUserLog( ErrorTranslator.class ), logging.getInternalLog( ErrorTranslator.class ));
+        this(logging.getUserLog( ErrorTranslator.class ) );
     }
 
-    public ErrorTranslator( Log userLog, Log internalLog )
+    public ErrorTranslator( Log userLog )
     {
-        this.internalLog = internalLog;
         this.userLog = userLog;
     }
 
@@ -72,16 +70,16 @@ public Neo4jError translate( Throwable any )
         // Log unknown errors.
         userLog.error( String.format(
                 "Client triggered unexpected error. Help us fix this error by emailing the " +
-                "following report to issues@neotechnology.com: %n%s", generateReport( any, reference ) ), any );
-        internalLog.error( String.format( "Client triggered unexpected error, reference %s.", reference), any );
+                "following report to issues@neotechnology.com: %n%s", generateReport( any, reference ) ) );
 
         return new Neo4jError( Status.General.UnknownFailure,
                 String.format( "An unexpected failure occurred, see details in the database " +
                                "logs, reference number %s.%n%s", reference, Exceptions.stringify( any ) ) );
     }
 
     /**
-     * For unexpected errors, we generate a report and urge users to send it to us for debugging.
+     * For unrecognized errors, we generate a report and urge users to send it to us for debugging. The report just contains some basic system info
+     * and a base64-encoded stack trace.
      */
     private String generateReport( Throwable err, String reference )
     {
@@ -98,15 +96,16 @@ private String generateReport( Throwable err, String reference )
                               "OS            : %s%n" +
                               "Reference     : %s%n" + // This will help us correlate emailed reports with user logs
                               "Error data    : %n" +
-                              "%s%n" +
+                              "%s" +
                               "---- END OF REPORT ----%n",
                 dateFormatGmt.format( new Date() ),
                 Version.getKernelVersion(),
                 System.getProperty("java.version"),
                 System.getProperty("os.name"),
                 reference,
                 printBase64Binary( Exceptions.stringify( err ).getBytes( StandardCharsets.UTF_8 ) )
-                        .replaceAll( "(.{100})", "$1\n" ) );
+                        // Below replaceAll call inserts a line break every 100 characters
+                        .replaceAll( "(.{100})", "$1" + System.getProperty("line.separator") ) );
     }
 
 }
diff --git a/community/ndp/runtime/src/test/java/org/neo4j/ndp/runtime/internal/ErrorTranslatorTest.java b/community/ndp/runtime/src/test/java/org/neo4j/ndp/runtime/internal/ErrorTranslatorTest.java
@@ -38,7 +38,7 @@ public void shouldReportUnknownErrors() throws Throwable
         // Given
         AssertableLogProvider provider = new AssertableLogProvider();
         ErrorTranslator translator =
-                new ErrorTranslator( provider.getLog( "userlog" ), provider.getLog( "internallog" ) );
+                new ErrorTranslator( provider.getLog( "userlog" ) );
 
         Throwable cause = new Throwable( "This is not an error we know how to handle." );
 
@@ -50,8 +50,6 @@ public void shouldReportUnknownErrors() throws Throwable
         provider.assertExactly(
                 inLog( "userlog" )
                     .error( both(containsString( "START OF REPORT" ))
-                            .and(containsString( "END OF REPORT" )), equalTo( cause ) ),
-                inLog( "internallog" )
-                    .error( containsString( "Client triggered unexpected error, reference" ), equalTo( cause ) ) );
+                            .and(containsString( "END OF REPORT" )) ));
     }
 }
diff --git a/community/server/src/main/java/org/neo4j/server/modules/DBMSModule.java b/community/server/src/main/java/org/neo4j/server/modules/DBMSModule.java
@@ -25,7 +25,7 @@
 import org.neo4j.server.rest.discovery.DiscoveryService;
 import org.neo4j.server.web.WebServer;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 /**
  * Mounts the DBMS REST API.
@@ -49,9 +49,9 @@ public void start()
 
     private List<String> getClassNames()
     {
-        return listFrom(
+        return asList(
                 DiscoveryService.class.getName(),
-                UserService.class.getName());
+                UserService.class.getName() );
     }
 
     @Override

diff --git a/community/server/src/main/java/org/neo4j/server/modules/ManagementApiModule.java b/community/server/src/main/java/org/neo4j/server/modules/ManagementApiModule.java
@@ -29,7 +29,7 @@
 import org.neo4j.server.web.ServerInternalSettings;
 import org.neo4j.server.web.WebServer;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 public class ManagementApiModule implements ServerModule
 {
@@ -51,7 +51,7 @@ public void start()
 
     private List<String> getClassNames()
     {
-        return listFrom(
+        return asList(
                 JmxService.class.getName(),
                 RootService.class.getName(),
                 VersionAndEditionService.class.getName() );

diff --git a/community/server/src/main/java/org/neo4j/server/modules/RESTApiModule.java b/community/server/src/main/java/org/neo4j/server/modules/RESTApiModule.java
@@ -46,7 +46,7 @@
 import org.neo4j.udc.UsageData;
 import org.neo4j.udc.UsageDataKeys;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 /**
  * Mounts the database REST API.
@@ -107,7 +107,7 @@ private RecentK<String> clientNames()
 
     private List<String> getClassNames()
     {
-        return listFrom(
+        return asList(
                 RestfulGraphDatabase.class.getName(),
                 TransactionalService.class.getName(),
                 CypherService.class.getName(),

diff --git a/community/server/src/main/java/org/neo4j/server/modules/ThirdPartyJAXRSModule.java b/community/server/src/main/java/org/neo4j/server/modules/ThirdPartyJAXRSModule.java
@@ -31,7 +31,7 @@
 import org.neo4j.server.plugins.Injectable;
 import org.neo4j.server.web.WebServer;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 public class ThirdPartyJAXRSModule implements ServerModule
 {
@@ -66,7 +66,7 @@ public void start()
 
     private List<String> packagesFor( ThirdPartyJaxRsPackage tpp )
     {
-        return listFrom( new String[] { tpp.getPackageName() } );
+        return asList( tpp.getPackageName() );
     }
 
     @Override

diff --git a/community/server/src/main/java/org/neo4j/server/modules/WebAdminModule.java b/community/server/src/main/java/org/neo4j/server/modules/WebAdminModule.java
@@ -28,7 +28,7 @@
 import org.neo4j.server.web.ServerInternalSettings;
 import org.neo4j.server.web.WebServer;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 public class WebAdminModule implements ServerModule
 {
@@ -57,7 +57,7 @@ public void start()
 
     private List<String> getClassNames()
     {
-        return listFrom(
+        return asList(
                 MonitorService.class.getName(),
                 ConsoleService.class.getName() );
     }

diff --git a/community/server/src/main/java/org/neo4j/server/web/JaxRsServletHolderFactory.java b/community/server/src/main/java/org/neo4j/server/web/JaxRsServletHolderFactory.java
@@ -19,21 +19,20 @@
  */
 package org.neo4j.server.web;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-
 import com.sun.jersey.api.core.ClassNamesResourceConfig;
 import com.sun.jersey.api.core.PackagesResourceConfig;
 import com.sun.jersey.api.core.ResourceConfig;
 import com.sun.jersey.spi.container.servlet.ServletContainer;
 import org.eclipse.jetty.servlet.ServletHolder;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
 import org.neo4j.server.database.InjectableProvider;
 import org.neo4j.server.modules.ServerModule;
 import org.neo4j.server.plugins.Injectable;
 import org.neo4j.server.rest.web.AllowAjaxFilter;
-import org.neo4j.server.rest.web.CollectUserAgentFilter;
 
 /**
  * Different {@link ServerModule}s can register services at the same mount point.
@@ -74,7 +73,7 @@ public ServletHolder create( Collection<InjectableProvider<?>> defaultInjectable
     private String getRequestFilterConfig()
     {
         // Ordering of execution of filters goes from left to right
-        return XForwardFilter.class.getName() + "," + CollectUserAgentFilter.class.getName();
+        return XForwardFilter.class.getName();
     }
 
     protected abstract void configure( ServletHolder servletHolder, String commaSeparatedList );

diff --git a/...erver-enterprise/src/main/java/org/neo4j/server/webadmin/rest/MasterInfoServerModule.java b/...erver-enterprise/src/main/java/org/neo4j/server/webadmin/rest/MasterInfoServerModule.java
@@ -29,7 +29,7 @@
 import org.neo4j.server.web.ServerInternalSettings;
 import org.neo4j.server.web.WebServer;
 
-import static org.neo4j.server.JAXRSHelper.listFrom;
+import static java.util.Arrays.asList;
 
 public class MasterInfoServerModule implements ServerModule
 {
@@ -62,7 +62,7 @@ public void stop()
 
     private List<String> getClassNames()
     {
-        return listFrom( MasterInfoService.class.getName() );
+        return asList( MasterInfoService.class.getName() );
     }
 
     private URI managementApiUri()