Some Small logging cleanup
- Use server error reporting in RF
- Some documentation fixes
- Create inherits module for projects using logging classes but not enabling loggin
Review at http://gwt-code-reviews.appspot.com/875802
Review by: amitmanjhi@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8849 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml b/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml
new file mode 100644
index 0000000..6caf995
--- /dev/null
+++ b/user/src/com/google/gwt/logging/LoggingDisabled.gwt.xml
@@ -0,0 +1,23 @@
+<!-- -->
+<!-- Copyright 2010 Google Inc. -->
+<!-- Licensed under the Apache License, Version 2.0 (the "License"); you -->
+<!-- may not use this file except in compliance with the License. You may -->
+<!-- may obtain a copy of the License at -->
+<!-- -->
+<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
+<!-- -->
+<!-- Unless required by applicable law or agreed to in writing, software -->
+<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
+<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -->
+<!-- implied. License for the specific language governing permissions and -->
+<!-- limitations under the License. -->
+
+
+<!-- Leaving out the inheritance of gwt.logging.Logging is enough to
+ disable logging, but some projects need the classes to be available to
+ avoid compile errors, even if they are disabling logging. Inheriting
+ this module will work for those projects. -->
+<module>
+ <inherits name="com.google.gwt.logging.Logging"/>
+ <set-property name="gwt.logging.enabled" value="FALSE"/>
+</module>
\ No newline at end of file
diff --git a/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java b/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java
index 61ded7d..0ddfefa 100644
--- a/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java
+++ b/user/src/com/google/gwt/logging/server/RemoteLoggingServiceImpl.java
@@ -39,7 +39,7 @@
/**
* Logs a Log Record which has been serialized using GWT RPC on the server.
- * Returns either an error message, or null if logging is successful.
+ * @return either an error message, or null if logging is successful.
*/
public final String logOnServer(SerializableLogRecord slr) {
String strongName = getPermutationStrongName();
@@ -48,7 +48,7 @@
slr, strongName, deobfuscator, loggerNameOverride);
} catch (RemoteLoggingException e) {
logger.log(Level.SEVERE, "Remote logging failed", e);
- return "Remote logging failed";
+ return "Remote logging failed, check stack trace for details.";
}
return null;
}
diff --git a/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml b/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml
index ca52699..0c0dfc5 100644
--- a/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml
+++ b/user/src/com/google/gwt/requestfactory/RequestFactory.gwt.xml
@@ -19,8 +19,7 @@
<inherits name='com.google.gwt.core.Core'/>
<inherits name='com.google.gwt.editor.Editor'/>
<inherits name='com.google.gwt.http.HTTP'/>
- <inherits name='com.google.gwt.logging.Logging'/>
- <set-property name="gwt.logging.enabled" value="FALSE"/>
+ <inherits name='com.google.gwt.logging.LoggingDisabled'/>
<source path="client"/>
<source path="shared"/>
diff --git a/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java b/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
index 13464d7..15c18e3 100644
--- a/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
+++ b/user/src/com/google/gwt/requestfactory/client/RequestFactoryLogHandler.java
@@ -38,17 +38,6 @@
LoggingRequest getLoggingRequest();
}
- private class LoggingReceiver extends Receiver<Long> {
- @Override
- public void onSuccess(Long response) {
- if (response > 0) {
- wireLogger.finest("Remote logging successful");
- } else {
- wireLogger.severe("Remote logging failed");
- }
- }
- }
-
private LoggingRequestProvider requestProvider;
/**
@@ -76,12 +65,10 @@
new SerializableLogRecord(record);
String json = JsonLogRecordClientUtil.serializableLogRecordAsJson(slr);
requestProvider.getLoggingRequest().logMessage(json).fire(
- new Receiver<String>() {
+ new Receiver<Void>() {
@Override
- public void onSuccess(String response) {
- if (!response.isEmpty()) {
- wireLogger.severe("Remote Logging failed on server: " + response);
- }
+ public void onSuccess(Void response) {
+ // Do nothing
}
});
}
diff --git a/user/src/com/google/gwt/requestfactory/server/Logging.java b/user/src/com/google/gwt/requestfactory/server/Logging.java
index 603064b..c72bb7f 100644
--- a/user/src/com/google/gwt/requestfactory/server/Logging.java
+++ b/user/src/com/google/gwt/requestfactory/server/Logging.java
@@ -21,9 +21,6 @@
import com.google.gwt.logging.server.StackTraceDeobfuscator;
import com.google.gwt.user.client.rpc.RpcRequestBuilder;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-
/**
* Server side object that handles log messages sent by
* {@link RequestFactoryLogHandler}.
@@ -36,24 +33,15 @@
private static StackTraceDeobfuscator deobfuscator =
new StackTraceDeobfuscator("");
- private static Logger logger = Logger.getLogger(Logging.class.getName());
-
- public static String logMessage(String serializedLogRecordJson) {
+ public static void logMessage(String serializedLogRecordJson) throws
+ RemoteLoggingException {
// if the header does not exist, we pass null, which is handled gracefully
// by the deobfuscation code.
String strongName =
RequestFactoryServlet.getThreadLocalRequest().getHeader(
RpcRequestBuilder.STRONG_NAME_HEADER);
- try {
- RemoteLoggingServiceUtil.logOnServer(serializedLogRecordJson,
- strongName, deobfuscator, null);
- } catch (RemoteLoggingException e) {
- // TODO(unnurg): Change this to use server failure reporting when it is
- // submitted.
- logger.log(Level.SEVERE, "Remote logging failed", e);
- return "Remote logging failed";
- }
- return "";
+ RemoteLoggingServiceUtil.logOnServer(serializedLogRecordJson,
+ strongName, deobfuscator, null);
}
/**
diff --git a/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java b/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java
index 49cb8e1..5aaf921 100644
--- a/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java
+++ b/user/src/com/google/gwt/requestfactory/shared/LoggingRequest.java
@@ -28,9 +28,7 @@
// TODO(unnurg): Pass a SerializableLogRecord here rather than it's
// serialized string.
/**
- * Log a message on the server. Will return empty string if there is no error
- * or an error message if there is a problem.
+ * Log a message on the server.
*/
- Request<String> logMessage(String serializedLogRecordString);
-
+ Request<Void> logMessage(String serializedLogRecordString);
}
diff --git a/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java b/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java
index ef2f6fb..8b9a681 100644
--- a/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java
+++ b/user/test/com/google/gwt/requestfactory/client/RequestFactoryExceptionHandlerTest.java
@@ -15,13 +15,8 @@
*/
package com.google.gwt.requestfactory.client;
-import com.google.gwt.requestfactory.shared.Receiver;
import com.google.gwt.requestfactory.shared.Request;
-import com.google.gwt.requestfactory.shared.ServerFailure;
import com.google.gwt.requestfactory.shared.SimpleFooProxy;
-import com.google.gwt.requestfactory.shared.Violation;
-
-import java.util.Set;
/**
* Tests that {@code RequestFactoryServlet} when using a custom
@@ -35,36 +30,29 @@
}
@Override
- public void testServerFailure() {
+ public void testServerFailureCheckedException() {
delayTestFinish(5000);
-
SimpleFooProxy rayFoo = req.create(SimpleFooProxy.class);
final Request<SimpleFooProxy> persistRay = req.simpleFooRequest().persistAndReturnSelf(
rayFoo);
rayFoo = persistRay.edit(rayFoo);
// 42 is the crash causing magic number
rayFoo.setPleaseCrash(42);
-
- persistRay.fire(new Receiver<SimpleFooProxy>() {
- @Override
- public void onFailure(ServerFailure error) {
- assertEquals("THIS EXCEPTION IS EXPECTED BY A TEST", error.getMessage());
- assertEquals("java.lang.UnsupportedOperationException",
- error.getExceptionType());
- assertFalse(error.getStackTraceString().length() == 0);
- finishTestAndReset();
- }
-
- @Override
- public void onViolation(Set<Violation> errors) {
- fail("Failure expected but onViolation() was called");
- }
-
- @Override
- public void onSuccess(SimpleFooProxy response) {
- fail("Failure expected but onSuccess() was called");
- }
- });
+ persistRay.fire(new FooReciever(rayFoo, persistRay,
+ "java.lang.UnsupportedOperationException"));
+ }
+
+ @Override
+ public void testServerFailureRuntimeException() {
+ delayTestFinish(5000);
+ SimpleFooProxy rayFoo = req.create(SimpleFooProxy.class);
+ final Request<SimpleFooProxy> persistRay = req.simpleFooRequest().persistAndReturnSelf(
+ rayFoo);
+ rayFoo = persistRay.edit(rayFoo);
+ // 43 is the crash causing magic number
+ rayFoo.setPleaseCrash(43);
+ persistRay.fire(new FooReciever(rayFoo, persistRay,
+ "java.lang.Exception"));
}
}
diff --git a/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java b/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
index c90fce3..b6ec204 100644
--- a/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
+++ b/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java
@@ -1020,48 +1020,76 @@
});
}
- public void testServerFailure() {
- delayTestFinish(5000);
+
+ class FooReciever extends Receiver<SimpleFooProxy> {
+ private SimpleFooProxy mutableFoo;
+ private Request<SimpleFooProxy> persistRequest;
+ private String expectedException;
+
+ public FooReciever(SimpleFooProxy mutableFoo,
+ Request<SimpleFooProxy> persistRequest, String exception) {
+ this.mutableFoo = mutableFoo;
+ this.persistRequest = persistRequest;
+ this.expectedException = exception;
+ }
- SimpleFooProxy newFoo = req.create(SimpleFooProxy.class);
- final Request<SimpleFooProxy> persistRequest = req.simpleFooRequest().persistAndReturnSelf(
- newFoo);
-
- final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo);
- mutableFoo.setPleaseCrash(42); // 42 is the crash causing magic number
-
- persistRequest.fire(new Receiver<SimpleFooProxy>() {
- @Override
- public void onFailure(ServerFailure error) {
+ @Override
+ public void onFailure(ServerFailure error) {
+ assertEquals(expectedException, error.getExceptionType());
+ if (expectedException.length() > 0) {
+ assertFalse(error.getStackTraceString().length() == 0);
+ assertEquals("THIS EXCEPTION IS EXPECTED BY A TEST",
+ error.getMessage());
+ } else {
+ assertEquals("", error.getStackTraceString());
assertEquals("Server Error: THIS EXCEPTION IS EXPECTED BY A TEST",
error.getMessage());
- assertEquals("", error.getExceptionType());
- assertEquals("", error.getStackTraceString());
-
- // Now show that we can fix the error and try again with the same
- // request
-
- mutableFoo.setPleaseCrash(24); // Only 42 crashes
- persistRequest.fire(new Receiver<SimpleFooProxy>() {
- @Override
- public void onSuccess(SimpleFooProxy response) {
- finishTestAndReset();
- }
- });
}
- @Override
- public void onSuccess(SimpleFooProxy response) {
- fail("Failure expected but onSuccess() was called");
- }
+ // Now show that we can fix the error and try again with the same
+ // request
- @Override
- public void onViolation(Set<Violation> errors) {
- fail("Failure expected but onViolation() was called");
- }
- });
+ mutableFoo.setPleaseCrash(24); // Only 42 and 43 crash
+ persistRequest.fire(new Receiver<SimpleFooProxy>() {
+ @Override
+ public void onSuccess(SimpleFooProxy response) {
+ finishTestAndReset();
+ }
+ });
+ }
+
+ @Override
+ public void onSuccess(SimpleFooProxy response) {
+ fail("Failure expected but onSuccess() was called");
+ }
+
+ @Override
+ public void onViolation(Set<Violation> errors) {
+ fail("Failure expected but onViolation() was called");
+ }
}
-
+
+ public void testServerFailureCheckedException() {
+ SimpleFooProxy newFoo = req.create(SimpleFooProxy.class);
+ final Request<SimpleFooProxy> persistRequest =
+ req.simpleFooRequest().persistAndReturnSelf(newFoo);
+ final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo);
+ // 43 is the crash causing magic number for a checked exception
+ mutableFoo.setPleaseCrash(43);
+ persistRequest.fire(new FooReciever(mutableFoo, persistRequest, ""));
+ }
+
+ public void testServerFailureRuntimeException() {
+ delayTestFinish(5000);
+ SimpleFooProxy newFoo = req.create(SimpleFooProxy.class);
+ final Request<SimpleFooProxy> persistRequest =
+ req.simpleFooRequest().persistAndReturnSelf(newFoo);
+ final SimpleFooProxy mutableFoo = persistRequest.edit(newFoo);
+ // 42 is the crash causing magic number for a runtime exception
+ mutableFoo.setPleaseCrash(42);
+ persistRequest.fire(new FooReciever(mutableFoo, persistRequest, ""));
+ }
+
public void testStableId() {
delayTestFinish(5000);
diff --git a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
index 54ef3fe..bc253a5 100644
--- a/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
+++ b/user/test/com/google/gwt/requestfactory/server/SimpleFoo.java
@@ -437,11 +437,14 @@
this.otherBoolField = otherBoolField;
}
- public void setPleaseCrash(Integer crashIf42) {
- if (crashIf42 == 42) {
+ public void setPleaseCrash(Integer crashIf42or43) throws Exception {
+ if (crashIf42or43 == 42) {
throw new UnsupportedOperationException("THIS EXCEPTION IS EXPECTED BY A TEST");
}
- pleaseCrash = crashIf42;
+ if (crashIf42or43 == 43) {
+ throw new Exception("THIS EXCEPTION IS EXPECTED BY A TEST");
+ }
+ pleaseCrash = crashIf42or43;
}
public void setPassword(String password) {
