Merge pull request ReactiveX#2632 from simonbasle/plugErrorRenderer

Implement hook to render specific types in OnNextValue

Author: benjchristensen

src/main/java/rx/exceptions/OnErrorThrowable.java

@@ -15,6 +15,9 @@
  */
 package rx.exceptions;
 
+import rx.plugins.RxJavaErrorHandler;
+import rx.plugins.RxJavaPlugins;
+
 /**
  * Represents a {@code Throwable} that an {@code Observable} might notify its subscribers of, but that then can
  * be handled by an operator that is designed to recover from or react appropriately to such an error. You can
@@ -106,6 +109,7 @@ public static Throwable addValueAsLastCause(Throwable e, Object value) {
     public static class OnNextValue extends RuntimeException {
 
         private static final long serialVersionUID = -3454462756050397899L;
+
         private final Object value;
 
         /**
@@ -131,11 +135,18 @@ public Object getValue() {
 
         /**
          * Render the object if it is a basic type. This avoids the library making potentially expensive
-         * or calls to toString() which may throw exceptions. See PR #1401 for details.
+         * or calls to toString() which may throw exceptions.
+         *
+         * If a specific behavior has been defined in the {@link RxJavaErrorHandler} plugin, some types
+         * may also have a specific rendering. Non-primitive types not managed by the plugin are rendered
+         * as the classname of the object.
+         * <p>
+         * See PR #1401 and Issue #2468 for details.
          *
          * @param value
          *        the item that the Observable was trying to emit at the time of the exception
-         * @return a string version of the object if primitive, otherwise the classname of the object
+         * @return a string version of the object if primitive or managed through error plugin,
+         *        otherwise the classname of the object
          */
         private static String renderValue(Object value){
             if (value == null) {
@@ -150,6 +161,12 @@ private static String renderValue(Object value){
             if (value instanceof Enum) {
                 return ((Enum<?>) value).name();
             }
+
+            String pluggedRendering = RxJavaPlugins.getInstance().getErrorHandler().handleOnNextValueRendering(value);
+            if (pluggedRendering != null) {
+                return pluggedRendering;
+            }
+
             return value.getClass().getName() + ".class";
         }
     }

src/main/java/rx/plugins/RxJavaErrorHandler.java

@@ -17,6 +17,9 @@
 
 import rx.Observable;
 import rx.Subscriber;
+import rx.annotations.Experimental;
+import rx.exceptions.Exceptions;
+import rx.exceptions.OnErrorThrowable;
 
 /**
  * Abstract class for defining error handling logic in addition to the normal
@@ -25,6 +28,8 @@
  * For example, all {@code Exception}s can be logged using this handler even if
  * {@link Subscriber#onError(Throwable)} is ignored or not provided when an {@link Observable} is subscribed to.
  * <p>
+ * This plugin is also responsible for augmenting rendering of {@link OnErrorThrowable.OnNextValue}.
+ * <p>
  * See {@link RxJavaPlugins} or the RxJava GitHub Wiki for information on configuring plugins: <a
  * href="https://github.com/ReactiveX/RxJava/wiki/Plugins">https://github.com/ReactiveX/RxJava/wiki/Plugins</a>.
  */
@@ -44,4 +49,52 @@ public void handleError(Throwable e) {
         // do nothing by default
     }
 
+    protected static final String ERROR_IN_RENDERING_SUFFIX = ".errorRendering";
+
+    /**
+     * Receives items causing {@link OnErrorThrowable.OnNextValue} and gives a chance to choose the String
+     * representation of the item in the OnNextValue stacktrace rendering. Returns null if this type of item
+     * is not managed and should use default rendering.
+     * <p>
+     * Note that primitive types are always rendered as their toString() value.
+     * <p>
+     * If a {@code Throwable} is caught when rendering, this will fallback to the item's classname suffixed by
+     * {@value #ERROR_IN_RENDERING_SUFFIX}.
+     *
+     * @param item the last emitted item, that caused the exception wrapped in {@link OnErrorThrowable.OnNextValue}.
+     * @return a short {@link String} representation of the item if one is known for its type, or null for default.
+     */
+    @Experimental
+    public final String handleOnNextValueRendering(Object item) {
+
+        try {
+            return render(item);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        } catch (Throwable t) {
+            Exceptions.throwIfFatal(t);
+        }
+        return item.getClass().getName() + ERROR_IN_RENDERING_SUFFIX;
+    }
+
+    /**
+     * Override this method to provide rendering for specific types other than primitive types and null.
+     * <p>
+     * For performance and overhead reasons, this should should limit to a safe production of a short {@code String}
+     * (as large renderings will bloat up the stacktrace). Prefer to try/catch({@code Throwable}) all code
+     * inside this method implementation.
+     * <p>
+     * If a {@code Throwable} is caught when rendering, this will fallback to the item's classname suffixed by
+     * {@value #ERROR_IN_RENDERING_SUFFIX}.
+     *
+     * @param item the last emitted item, that caused the exception wrapped in {@link OnErrorThrowable.OnNextValue}.
+     * @return a short {@link String} representation of the item if one is known for its type, or null for default.
+     * @throws InterruptedException if the rendering thread is interrupted
+     */
+    @Experimental
+    protected String render (Object item) throws InterruptedException {
+        //do nothing by default
+        return null;
+    }
+
 }

src/test/java/rx/plugins/RxJavaPluginsTest.java

@@ -16,16 +16,25 @@
 package rx.plugins;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import java.util.Calendar;
+import java.util.Collections;
+import java.util.Date;
+import java.util.concurrent.TimeUnit;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 import rx.Observable;
 import rx.Subscriber;
+import rx.exceptions.OnErrorThrowable;
+import rx.functions.Func1;
 
 public class RxJavaPluginsTest {
 
@@ -78,7 +87,18 @@ public void handleError(Throwable e) {
             this.e = e;
             count++;
         }
+    }
 
+    public static class RxJavaErrorHandlerTestImplWithRender extends RxJavaErrorHandler {
+        @Override
+        protected String render(Object item) {
+            if (item instanceof Calendar) {
+                throw new IllegalArgumentException("calendar");
+            } else if (item instanceof Date) {
+                return String.valueOf(((Date) item).getTime());
+            }
+            return null;
+        }
     }
 
     @Test
@@ -149,12 +169,86 @@ public void testOnErrorWhenNotImplemented() {
         assertEquals(1, errorHandler.count);
     }
 
+    @Test
+    public void testOnNextValueRenderingWhenNotImplemented() {
+        RxJavaErrorHandlerTestImpl errorHandler = new RxJavaErrorHandlerTestImpl();
+        RxJavaPlugins.getInstance().registerErrorHandler(errorHandler);
+
+        String rendering = RxJavaPlugins.getInstance().getErrorHandler().handleOnNextValueRendering(new Date());
+
+        assertNull(rendering);
+    }
+
+    @Test
+    public void testOnNextValueRenderingWhenImplementedAndNotManaged() {
+        RxJavaErrorHandlerTestImplWithRender errorHandler = new RxJavaErrorHandlerTestImplWithRender();
+        RxJavaPlugins.getInstance().registerErrorHandler(errorHandler);
+
+        String rendering = RxJavaPlugins.getInstance().getErrorHandler().handleOnNextValueRendering(
+                Collections.emptyList());
+
+        assertNull(rendering);
+    }
+
+    @Test
+    public void testOnNextValueRenderingWhenImplementedAndManaged() {
+        RxJavaErrorHandlerTestImplWithRender errorHandler = new RxJavaErrorHandlerTestImplWithRender();
+        RxJavaPlugins.getInstance().registerErrorHandler(errorHandler);
+        long time = 1234L;
+        Date date = new Date(time);
+
+        String rendering = RxJavaPlugins.getInstance().getErrorHandler().handleOnNextValueRendering(date);
+
+        assertNotNull(rendering);
+        assertEquals(String.valueOf(time), rendering);
+    }
+
+    @Test
+    public void testOnNextValueRenderingWhenImplementedAndThrows() {
+        RxJavaErrorHandlerTestImplWithRender errorHandler = new RxJavaErrorHandlerTestImplWithRender();
+        RxJavaPlugins.getInstance().registerErrorHandler(errorHandler);
+        Calendar cal = Calendar.getInstance();
+
+        String rendering = RxJavaPlugins.getInstance().getErrorHandler().handleOnNextValueRendering(cal);
+
+        assertNotNull(rendering);
+        assertEquals(cal.getClass().getName() + RxJavaErrorHandler.ERROR_IN_RENDERING_SUFFIX, rendering);
+    }
+
+    @Test
+    public void testOnNextValueCallsPlugin() {
+        RxJavaErrorHandlerTestImplWithRender errorHandler = new RxJavaErrorHandlerTestImplWithRender();
+        RxJavaPlugins.getInstance().registerErrorHandler(errorHandler);
+        long time = 456L;
+        Date date = new Date(time);
+
+        try {
+            Date notExpected = Observable.just(date)
+                                         .map(new Func1<Date, Date>() {
+                                             @Override
+                                             public Date call(Date date) {
+                                                 throw new IllegalStateException("Trigger OnNextValue");
+                                             }
+                                         })
+                                         .timeout(500, TimeUnit.MILLISECONDS)
+                                         .toBlocking().first();
+            fail("Did not expect onNext/onCompleted, got " + notExpected);
+        } catch (IllegalStateException e) {
+            assertEquals("Trigger OnNextValue", e.getMessage());
+            assertNotNull(e.getCause());
+            assertTrue(e.getCause() instanceof OnErrorThrowable.OnNextValue);
+            assertEquals("OnError while emitting onNext value: " + time, e.getCause().getMessage());
+        }
+
+    }
+
     // inside test so it is stripped from Javadocs
     public static class RxJavaObservableExecutionHookTestImpl extends RxJavaObservableExecutionHook {
         // just use defaults
     }
 
     private static String getFullClassNameForTestClass(Class<?> cls) {
-        return RxJavaPlugins.class.getPackage().getName() + "." + RxJavaPluginsTest.class.getSimpleName() + "$" + cls.getSimpleName();
+        return RxJavaPlugins.class.getPackage()
+                                  .getName() + "." + RxJavaPluginsTest.class.getSimpleName() + "$" + cls.getSimpleName();
     }
 }