unicorn commit: ~ logs stack traces in case of an unexpected internal error instead of displaying to the user.

changeset:   1620:d3e30d1d6470
tag:         tip
user:        Thomas Gambet <tgambet@w3.org>
date:        Thu Feb 24 10:58:08 2011 -0500
files:       WebContent/WEB-INF/conf/unicorn_log4j.xml.default WebContent/WEB-INF/languages/en.properties src/org/w3c/unicorn/RequestThread.java src/org/w3c/unicorn/action/ObserveAction.java src/org/w3c/unicorn/util/Message.java
description:
~ logs stack traces in case of an unexpected internal error instead of displaying to the user.


diff -r 087c633956da -r d3e30d1d6470 WebContent/WEB-INF/conf/unicorn_log4j.xml.default
--- a/WebContent/WEB-INF/conf/unicorn_log4j.xml.default	Thu Feb 24 10:21:54 2011 -0500
+++ b/WebContent/WEB-INF/conf/unicorn_log4j.xml.default	Thu Feb 24 10:58:08 2011 -0500
@@ -175,15 +175,6 @@
 		</layout>
 	</appender>	
 
-	<appender name="critical" class="org.apache.log4j.RollingFileAppender">
-		<param name="File" value="${unicorn.home}/WEB-INF/logs/critical.log" />
-		<param name="MaxFileSize" value="1024KB" />
-		<param name="MaxBackupIndex" value="1" />
-		<layout class="org.apache.log4j.PatternLayout">
-			<param name="ConversionPattern" value="%d{yyyy/MM/dd HH:mm:ss} %-5p (%F:%L) %m%n" />
-		</layout>
-	</appender>	
-
 	<logger name="org.apache.velocity">
 		<level value="debug"/>
 		<appender-ref ref="velocity_log" />
@@ -234,10 +225,6 @@
 		<appender-ref ref="package-util" />
 	</logger>
 	
-	<logger name="Critical">
-		<appender-ref ref="critical" />
-	</logger>
-	
 	<root>
 		<level value="info"/>
 		<appender-ref ref="level-error" />
diff -r 087c633956da -r d3e30d1d6470 WebContent/WEB-INF/languages/en.properties
--- a/WebContent/WEB-INF/languages/en.properties	Thu Feb 24 10:21:54 2011 -0500
+++ b/WebContent/WEB-INF/languages/en.properties	Thu Feb 24 10:58:08 2011 -0500
@@ -41,6 +41,7 @@
 message_input_changed_long=Some HTTP headers may have been lost or changed and the checker may not be able to follow the links contained in your document.
 message_invalid_mime_type=The specified mime-type (%1) is invalid.
 message_invalid_url_syntax=The specified URI is invalid: "%1".
+message_internal_error=Unicorn encountered an internal error and warned the maintainer automatically. Apologies for the inconvenience.
 message_local_address_provided=Local IP addresses are forbidden by Unicorn's configuration. Please use a public address.
 message_mail_date=Check executed the %1.
 message_mail=The report is being sent to: %1.
diff -r 087c633956da -r d3e30d1d6470 src/org/w3c/unicorn/RequestThread.java
--- a/src/org/w3c/unicorn/RequestThread.java	Thu Feb 24 10:21:54 2011 -0500
+++ b/src/org/w3c/unicorn/RequestThread.java	Thu Feb 24 10:58:08 2011 -0500
@@ -22,7 +22,6 @@
 	 * Used for complex logging purpose
 	 */
 	private static final Log logger = LogFactory.getLog(RequestThread.class);
-	private static final Log criticalLogger = LogFactory.getLog("CriticalError");
 	
 	private Response aResponse;
 
@@ -68,10 +67,7 @@
 			aResponse.setObserverId(obsID);
 		} catch (final UnicornException e) {
 			messages.add(e.getUnicornMessage());
-			criticalLogger.error("Observer request failed: \n\t Request: " +  aRequest.getInputMethod().toString() + " - " + this.aRequest.toString() + "\n " + e.getMessage());
-		} catch (final Exception e) {
-			messages.add(new Message(e));
-			logger.error(e.getMessage(), e);
+			logger.error("Observer request failed: \n\t Request: " +  aRequest.getInputMethod().toString() + " - " + this.aRequest.toString(), e);
 		}
 	}
 
diff -r 087c633956da -r d3e30d1d6470 src/org/w3c/unicorn/action/ObserveAction.java
--- a/src/org/w3c/unicorn/action/ObserveAction.java	Thu Feb 24 10:21:54 2011 -0500
+++ b/src/org/w3c/unicorn/action/ObserveAction.java	Thu Feb 24 10:58:08 2011 -0500
@@ -49,7 +49,6 @@
 	private static final long serialVersionUID = -1375355420965607571L;
 	
 	private static Log logger = LogFactory.getLog(ObserveAction.class);
-	private static Log criticalLogger = LogFactory.getLog("CriticalError");
 	
 	private static DiskFileItemFactory factory;
 	
@@ -312,26 +311,27 @@
 				resp.setHeader("X-W3C-Validator-Status", "Abort");
 			} else {
 				if (e.getUnicornMessage() != null) {
-					messages.add(e.getUnicornMessage());
+					if (!e.getUnicornMessage().isCritical())
+						messages.add(e.getUnicornMessage());
+					else {
+						StringBuilder log = new StringBuilder();
+						for (String key : reqParams.keySet()) {
+							log.append(key + " -> ");
+							if (reqParams.get(key) instanceof String[]) {
+								String[] params = (String[]) reqParams.get(key);
+								for (String param : params)
+									log.append(param + "\n");
+							} else {
+								log.append(reqParams.get(key) + "\n");
+							}
+						}
+						logger.error("Critical: \n" + log, e);
+						messages.add(new Message(Message.ERROR, "$message_internal_error", null));
+					}
 				} else if (e.getMessage() != null)
 					messages.add(new Message(Message.ERROR, e.getMessage(), null));
 				aOutputModule.produceError(mapOfStringObject, resp.getWriter());
 			}
-		} catch (final Exception aException) {
-			StringBuilder log = new StringBuilder();
-			for (String key : reqParams.keySet()) {
-				log.append(key + " -> ");
-				if (reqParams.get(key) instanceof String[]) {
-					String[] params = (String[]) reqParams.get(key);
-					for (String param : params)
-						log.append(param + "\n");
-				} else {
-					log.append(reqParams.get(key) + "\n");
-				}
-			}
-			criticalLogger.error("Critical: \n" + log, aException);
-			messages.add(new Message(aException));
-			aOutputModule.produceError(mapOfStringObject, resp.getWriter());
 		} finally {
 			aUnicornCall.dispose();
 			if ("true".equals(Property.get("DELETE_UPLOADED_FILES")) && aFileItemUploaded != null)
diff -r 087c633956da -r d3e30d1d6470 src/org/w3c/unicorn/util/Message.java
--- a/src/org/w3c/unicorn/util/Message.java	Thu Feb 24 10:21:54 2011 -0500
+++ b/src/org/w3c/unicorn/util/Message.java	Thu Feb 24 10:58:08 2011 -0500
@@ -13,7 +13,8 @@
 	private String content;
 	private int level;
 	private boolean evaluateContent = false;
-	
+	private boolean isCritical = false;
+
 	private String[] parameters;
 	
 	public Message() {
@@ -28,6 +29,7 @@
 		level = ERROR;
 		message = "$stack_trace_text";
 		evaluateContent = false;
+		isCritical = true;
 	}
 	
 	public Message(int level, String message, String content, String... parameters) {
@@ -79,4 +81,8 @@
 		this.evaluateContent = evaluateContent;
 	}
 	
+	public boolean isCritical() {
+		return isCritical;
+	}
+	
 }

Received on Thursday, 24 February 2011 15:58:40 UTC