Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement hook to render specific types in OnNextValue #2632

Merged
merged 3 commits into from
Mar 17, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/main/java/rx/exceptions/OnErrorThrowable.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

/**
Expand All @@ -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) {
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the ErrorHandler should be statically referenced once at the top of this class?

if (pluggedRendering != null) {
return pluggedRendering;
}

return value.getClass().getName() + ".class";
}
}
Expand Down
53 changes: 53 additions & 0 deletions src/main/java/rx/plugins/RxJavaErrorHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>.
*/
Expand All @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add @Experimental for right now during the 1.0.x release cycle?


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;
}

}
96 changes: 95 additions & 1 deletion src/test/java/rx/plugins/RxJavaPluginsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}