From d6a4838b6c6355048afbe10087a3ce73619bf03d Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 25 Aug 2018 18:35:09 +0100 Subject: [PATCH 01/10] [android] update Promise interface & to allow a userInfo WritableMap to be passed to reject --- .../com/facebook/react/bridge/Promise.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java index aa8c3bee4a83fb..69dd71513a2c3a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java @@ -28,16 +28,31 @@ public interface Promise { */ void reject(String code, String message); + /** + * Report an error which wasn't caused by an exception, with userInfo. + */ + void reject(String code, String message, WritableMap userInfo); + /** * Report an exception. */ void reject(String code, Throwable e); + /** + * Report an exception, with userInfo. + */ + void reject(String code, Throwable e, WritableMap userInfo); + /** * Report an exception with a custom error message. */ void reject(String code, String message, Throwable e); + /** + * Report an exception with a custom error message, with userInfo. + */ + void reject(String code, String message, Throwable e, WritableMap userInfo); + /** * Report an error which wasn't caused by an exception. * @deprecated Prefer passing a module-specific error code to JS. @@ -51,4 +66,10 @@ public interface Promise { * Useful in catch-all scenarios where it's unclear why the error occurred. */ void reject(Throwable reason); + + /** + * Report an exception, with default error code, with userInfo. + * Useful in catch-all scenarios where it's unclear why the error occurred. + */ + void reject(Throwable reason, WritableMap userInfo); } From 346b7330930c2ffbf23bd2bdfaf151e633dc7090 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 25 Aug 2018 18:35:42 +0100 Subject: [PATCH 02/10] [android] update Promise implementation to allow a userInfo WritableMap instance to be passed to reject --- .../facebook/react/bridge/PromiseImpl.java | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java index 1ee2380ff8bba6..bcd2ecbe704b9f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java @@ -33,27 +33,47 @@ public void resolve(Object value) { @Override public void reject(String code, String message) { - reject(code, message, /*Throwable*/null); + reject(code, message, /*Throwable*/null, /*WritableMap*/null); + } + + @Override + public void reject(String code, String message, WritableMap userInfo) { + reject(code, message, /*Throwable*/null, userInfo); } @Override @Deprecated public void reject(String message) { - reject(DEFAULT_ERROR, message, /*Throwable*/null); + reject(DEFAULT_ERROR, message, /*Throwable*/null, /*WritableMap*/null); } @Override public void reject(String code, Throwable e) { - reject(code, e.getMessage(), e); + reject(code, e.getMessage(), e, /*WritableMap*/null); + } + + @Override + public void reject(String code, Throwable e, WritableMap userInfo) { + reject(code, e.getMessage(), e, userInfo); } @Override public void reject(Throwable e) { - reject(DEFAULT_ERROR, e.getMessage(), e); + reject(DEFAULT_ERROR, e.getMessage(), e, /*WritableMap*/null); } @Override - public void reject(String code, String message, @Nullable Throwable e) { + public void reject(Throwable e, WritableMap userInfo) { + reject(DEFAULT_ERROR, e.getMessage(), e, userInfo); + } + + @Override + public void reject(String code, String message, Throwable e) { + reject(code, message, e, /*WritableMap*/null); + } + + @Override + public void reject(String code, String message, @Nullable Throwable e, @Nullable WritableMap userInfo) { if (mReject != null) { if (code == null) { code = DEFAULT_ERROR; @@ -64,6 +84,13 @@ public void reject(String code, String message, @Nullable Throwable e) { WritableNativeMap errorInfo = new WritableNativeMap(); errorInfo.putString("code", code); errorInfo.putString("message", message); + + if (userInfo != null) { + errorInfo.putMap("userInfo", userInfo); + } else { + errorInfo.putNull("userInfo"); + } + // TODO(8850038): add the stack trace info in, need to figure out way to serialize that mReject.invoke(errorInfo); } From 41e34bfd4049f7124791197829cb45d0ccadfcc7 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 25 Aug 2018 19:14:25 +0100 Subject: [PATCH 03/10] [android] remove inconsistent promise.reject with null arg - unnecessary and not done anywhere else - prevents a 'ambiguous method' error as rejects 3rd arg can be Throwable or Writable map. --- .../com/facebook/react/modules/netinfo/NetInfoModule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/netinfo/NetInfoModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/netinfo/NetInfoModule.java index 98c6a5f1761fa9..0f1352ee36b0f7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/netinfo/NetInfoModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/netinfo/NetInfoModule.java @@ -102,7 +102,7 @@ public String getName() { @ReactMethod public void getCurrentConnectivity(Promise promise) { if (mNoNetworkPermission) { - promise.reject(ERROR_MISSING_PERMISSION, MISSING_PERMISSION_MESSAGE, null); + promise.reject(ERROR_MISSING_PERMISSION, MISSING_PERMISSION_MESSAGE); return; } promise.resolve(createConnectivityEventMap()); @@ -111,7 +111,7 @@ public void getCurrentConnectivity(Promise promise) { @ReactMethod public void isConnectionMetered(Promise promise) { if (mNoNetworkPermission) { - promise.reject(ERROR_MISSING_PERMISSION, MISSING_PERMISSION_MESSAGE, null); + promise.reject(ERROR_MISSING_PERMISSION, MISSING_PERMISSION_MESSAGE); return; } promise.resolve(ConnectivityManagerCompat.isActiveNetworkMetered(mConnectivityManager)); From 6d0aa8e90b86988366d67c394206492d8ed6c838 Mon Sep 17 00:00:00 2001 From: Salakar Date: Sat, 25 Aug 2018 22:10:43 +0100 Subject: [PATCH 04/10] [android] fix failing unit tests build - missing buck dependency --- ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK | 1 + 1 file changed, 1 insertion(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK b/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK index d473c9cf2107a0..30293a45533d94 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK @@ -6,6 +6,7 @@ rn_android_library( proguard_config = "reactnative.pro", provided_deps = [ react_native_dep("third-party/android/support/v4:lib-support-v4"), + react_native_dep("third-party/android/support-annotations:android-support-annotations"), ], required_for_source_only_abi = True, visibility = [ From 87d4269d6cfd3c844667f614e120accecd11c7a0 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Sat, 1 Sep 2018 16:29:15 +0100 Subject: [PATCH 05/10] [android] cleanup Promise interface --- .../com/facebook/react/bridge/Promise.java | 102 +++++++++++++----- 1 file changed, 76 insertions(+), 26 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java index 69dd71513a2c3a..40bee3d0ccd805 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/Promise.java @@ -1,4 +1,4 @@ -/** +/* * Copyright (c) 2015-present, Facebook, Inc. * * This source code is licensed under the MIT license found in the @@ -7,69 +7,119 @@ package com.facebook.react.bridge; +import javax.annotation.Nonnull; import javax.annotation.Nullable; -/** +/* * Interface that represents a JavaScript Promise which can be passed to the native module as a * method parameter. * - * Methods annotated with {@link ReactMethod} that use {@link Promise} as type of the last parameter + * Methods annotated with {@link ReactMethod} that use a {@link Promise} as the last parameter * will be marked as "promise" and will return a promise when invoked from JavaScript. */ public interface Promise { /** - * Successfully resolve the Promise. + * Successfully resolve the Promise with an optional value. + * + * @param value Object */ void resolve(@Nullable Object value); /** - * Report an error which wasn't caused by an exception. + * Report an error without an exception using a custom code and error message. + * + * @param code String + * @param message String */ void reject(String code, String message); /** - * Report an error which wasn't caused by an exception, with userInfo. + * Report an exception with a custom code. + * + * @param code String + * @param throwable Throwable */ - void reject(String code, String message, WritableMap userInfo); + void reject(String code, Throwable throwable); /** - * Report an exception. + * Report an exception with a custom code and error message. + * + * @param code String + * @param message String + * @param throwable Throwable */ - void reject(String code, Throwable e); + void reject(String code, String message, Throwable throwable); + /** - * Report an exception, with userInfo. + * Report an exception, with default error code. + * Useful in catch-all scenarios where it's unclear why the error occurred. + * + * @param throwable Throwable */ - void reject(String code, Throwable e, WritableMap userInfo); + void reject(Throwable throwable); + + /* --------------------------- + * With userInfo WritableMap + * --------------------------- */ /** - * Report an exception with a custom error message. + * Report an exception, with default error code, with userInfo. + * Useful in catch-all scenarios where it's unclear why the error occurred. + * + * @param throwable Throwable + * @param userInfo WritableMap */ - void reject(String code, String message, Throwable e); + void reject(Throwable throwable, WritableMap userInfo); /** - * Report an exception with a custom error message, with userInfo. + * Reject with a code and userInfo WritableMap. + * + * @param code String + * @param userInfo WritableMap */ - void reject(String code, String message, Throwable e, WritableMap userInfo); + void reject(String code, @Nonnull WritableMap userInfo); /** - * Report an error which wasn't caused by an exception. - * @deprecated Prefer passing a module-specific error code to JS. - * Using this method will pass the error code "EUNSPECIFIED". + * Report an exception with a custom code and userInfo. + * + * @param code String + * @param throwable Throwable + * @param userInfo WritableMap */ - @Deprecated - void reject(String message); + void reject(String code, Throwable throwable, WritableMap userInfo); /** - * Report an exception, with default error code. - * Useful in catch-all scenarios where it's unclear why the error occurred. + * Report an error with a custom code, error message and userInfo, + * an error not caused by an exception. + * + * @param code String + * @param message String + * @param userInfo WritableMap */ - void reject(Throwable reason); + void reject(String code, String message, @Nonnull WritableMap userInfo); /** - * Report an exception, with default error code, with userInfo. - * Useful in catch-all scenarios where it's unclear why the error occurred. + * Report an exception with a custom code, error message and userInfo. + * + * @param code String + * @param message String + * @param throwable Throwable + * @param userInfo WritableMap */ - void reject(Throwable reason, WritableMap userInfo); + void reject(String code, String message, Throwable throwable, WritableMap userInfo); + + /* ------------ + * Deprecated + * ------------ */ + + /** + * Report an error which wasn't caused by an exception. + * + * @deprecated Prefer passing a module-specific error code to JS. + * Using this method will pass the error code "EUNSPECIFIED". + */ + @Deprecated + void reject(String message); } From 4036494e56100e9f19af80d2f462e001161ee7f2 Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Sat, 1 Sep 2018 16:32:53 +0100 Subject: [PATCH 06/10] [android] Promise implementation changes Cleanup to match the interface and additionally adds support/fixes for `TODO(8850038)` --- .../facebook/react/bridge/PromiseImpl.java | 220 ++++++++++++++---- 1 file changed, 180 insertions(+), 40 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java index bcd2ecbe704b9f..1a1d751918f385 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java @@ -1,29 +1,56 @@ -/** +/* * Copyright (c) 2015-present, Facebook, Inc. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. */ -/** - * Implementation of two javascript functions that can be used to resolve or reject a js promise. - */ package com.facebook.react.bridge; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +/* + * Implementation of {@link Promise} that represents a JavaScript Promise which can be passed to the + * native module as a method parameter. + * + * Methods annotated with {@link ReactMethod} that use a {@link Promise} as the last parameter + * will be marked as "promise" and will return a promise when invoked from JavaScript. + */ public class PromiseImpl implements Promise { + // Number of stack frames to parse and return to mReject.invoke + // for ERROR_MAP_KEY_NATIVE_STACK + private static final int ERROR_STACK_FRAME_LIMIT = 10; - private static final String DEFAULT_ERROR = "EUNSPECIFIED"; + private static final String ERROR_DEFAULT_CODE = "EUNSPECIFIED"; + private static final String ERROR_DEFAULT_MESSAGE = "Error not specified."; - private @Nullable Callback mResolve; - private @Nullable Callback mReject; + // Keys for mReject's WritableMap + private static final String ERROR_MAP_KEY_CODE = "code"; + private static final String ERROR_MAP_KEY_MESSAGE = "message"; + private static final String ERROR_MAP_KEY_USER_INFO = "userInfo"; + private static final String ERROR_MAP_KEY_NATIVE_STACK = "nativeStackAndroid"; + + // Keys for ERROR_MAP_KEY_NATIVE_STACK's StackFrame maps + private static final String STACK_FRAME_KEY_FILE = "file"; + private static final String STACK_FRAME_KEY_LINE_NUMBER = "lineNumber"; + private static final String STACK_FRAME_KEY_METHOD_NAME = "methodName"; + + private @Nullable + Callback mResolve; + private @Nullable + Callback mReject; public PromiseImpl(@Nullable Callback resolve, @Nullable Callback reject) { mResolve = resolve; mReject = reject; } + /** + * Successfully resolve the Promise with an optional value. + * + * @param value Object + */ @Override public void resolve(Object value) { if (mResolve != null) { @@ -31,68 +58,181 @@ public void resolve(Object value) { } } + /** + * Report an error without an exception using a custom code and error message. + * + * @param code String + * @param message String + */ @Override public void reject(String code, String message) { reject(code, message, /*Throwable*/null, /*WritableMap*/null); } + /** + * Report an exception with a custom code. + * + * @param code String + * @param throwable Throwable + */ @Override - public void reject(String code, String message, WritableMap userInfo) { - reject(code, message, /*Throwable*/null, userInfo); + public void reject(String code, Throwable throwable) { + reject(code, /*Message*/null, throwable, /*WritableMap*/null); } + /** + * Report an exception with a custom code and error message. + * + * @param code String + * @param message String + * @param throwable Throwable + */ @Override - @Deprecated - public void reject(String message) { - reject(DEFAULT_ERROR, message, /*Throwable*/null, /*WritableMap*/null); + public void reject(String code, String message, Throwable throwable) { + reject(code, message, throwable, /*WritableMap*/null); } + /** + * Report an exception, with default error code. + * Useful in catch-all scenarios where it's unclear why the error occurred. + * + * @param throwable Throwable + */ @Override - public void reject(String code, Throwable e) { - reject(code, e.getMessage(), e, /*WritableMap*/null); + public void reject(Throwable throwable) { + reject(/*Code*/null, /*Message*/null, throwable, /*WritableMap*/null); } + /* --------------------------- + * With userInfo WritableMap + * --------------------------- */ + + /** + * Report an exception, with default error code, with userInfo. + * Useful in catch-all scenarios where it's unclear why the error occurred. + * + * @param throwable Throwable + * @param userInfo WritableMap + */ @Override - public void reject(String code, Throwable e, WritableMap userInfo) { - reject(code, e.getMessage(), e, userInfo); + public void reject(Throwable throwable, WritableMap userInfo) { + reject(/*Code*/null, /*Message*/null, throwable, userInfo); } + /** + * Reject with a code and userInfo WritableMap. + * + * @param code String + * @param userInfo WritableMap + */ @Override - public void reject(Throwable e) { - reject(DEFAULT_ERROR, e.getMessage(), e, /*WritableMap*/null); + public void reject(String code, @Nonnull WritableMap userInfo) { + reject(code, /*Message*/null, /*Throwable*/null, userInfo); } + /** + * Report an exception with a custom code and userInfo. + * + * @param code String + * @param throwable Throwable + * @param userInfo WritableMap + */ @Override - public void reject(Throwable e, WritableMap userInfo) { - reject(DEFAULT_ERROR, e.getMessage(), e, userInfo); + public void reject(String code, Throwable throwable, WritableMap userInfo) { + reject(code, /*Message*/null, throwable, userInfo); } + /** + * Report an error with a custom code, error message and userInfo, + * an error not caused by an exception. + * + * @param code String + * @param message String + * @param userInfo WritableMap + */ @Override - public void reject(String code, String message, Throwable e) { - reject(code, message, e, /*WritableMap*/null); + public void reject(String code, String message, @Nonnull WritableMap userInfo) { + reject(code, message, /*Throwable*/null, userInfo); } + /** + * Report an exception with a custom code, error message and userInfo. + * + * @param code String + * @param message String + * @param throwable Throwable + * @param userInfo WritableMap + */ @Override - public void reject(String code, String message, @Nullable Throwable e, @Nullable WritableMap userInfo) { - if (mReject != null) { - if (code == null) { - code = DEFAULT_ERROR; - } - // The JavaScript side expects a map with at least the error message. - // It is possible to expose all kind of information. It will be available on the JS - // error instance. - WritableNativeMap errorInfo = new WritableNativeMap(); - errorInfo.putString("code", code); - errorInfo.putString("message", message); - - if (userInfo != null) { - errorInfo.putMap("userInfo", userInfo); - } else { - errorInfo.putNull("userInfo"); + public void reject( + @Nullable String code, + @Nullable String message, + @Nullable Throwable throwable, + @Nullable WritableMap userInfo + ) { + if (mReject == null) return; + WritableNativeMap errorInfo = new WritableNativeMap(); + + if (code == null) { + errorInfo.putString(ERROR_MAP_KEY_CODE, ERROR_DEFAULT_CODE); + } else { + errorInfo.putString(ERROR_MAP_KEY_CODE, code); + } + + // Use the custom message if provided otherwise use the throwable message. + if (message != null) { + errorInfo.putString(ERROR_MAP_KEY_MESSAGE, message); + } else if (throwable != null) { + errorInfo.putString(ERROR_MAP_KEY_MESSAGE, throwable.getMessage()); + } else { + // The JavaScript side expects a map with at least an error message. + // /Libraries/BatchedBridge/NativeModules.js -> createErrorFromErrorData + // TYPE: (errorData: { message: string }) + errorInfo.putString(ERROR_MAP_KEY_MESSAGE, ERROR_DEFAULT_MESSAGE); + } + + // For consistency with iOS ensure userInfo key exists, even if we null it. + // iOS: /React/Base/RCTUtils.m -> RCTJSErrorFromCodeMessageAndNSError + if (userInfo != null) { + errorInfo.putMap(ERROR_MAP_KEY_USER_INFO, userInfo); + } else { + errorInfo.putNull(ERROR_MAP_KEY_USER_INFO); + } + + // Attach a nativeStackAndroid array if a throwable was passed + // this matches iOS behavior - iOS adds a `nativeStackIOS` property + // iOS: /React/Base/RCTUtils.m -> RCTJSErrorFromCodeMessageAndNSError + if (throwable != null) { + StackTraceElement[] stackTrace = throwable.getStackTrace(); + WritableNativeArray nativeStackAndroid = new WritableNativeArray(); + + // Build an an Array of StackFrames to match JavaScript: + // iOS: /Libraries/Core/Devtools/parseErrorStack.js -> StackFrame + for (int i = 0; i < ERROR_STACK_FRAME_LIMIT; i++) { + StackTraceElement frame = stackTrace[i]; + WritableMap frameMap = new WritableNativeMap(); + // NOTE: no column number exists StackTraceElement + frameMap.putString(STACK_FRAME_KEY_FILE, frame.getFileName()); + frameMap.putInt(STACK_FRAME_KEY_LINE_NUMBER, frame.getLineNumber()); + frameMap.putString(STACK_FRAME_KEY_METHOD_NAME, frame.getMethodName()); + nativeStackAndroid.pushMap(frameMap); } - // TODO(8850038): add the stack trace info in, need to figure out way to serialize that - mReject.invoke(errorInfo); + errorInfo.putArray(ERROR_MAP_KEY_NATIVE_STACK, nativeStackAndroid); + } else { + errorInfo.putArray(ERROR_MAP_KEY_NATIVE_STACK, new WritableNativeArray()); } + + mReject.invoke(errorInfo); + } + + /* ------------ + * Deprecated + * ------------ */ + + @Override + @Deprecated + public void reject(String message) { + reject(/*Code*/null, message, /*Throwable*/null, /*WritableMap*/null); } } From e4c7766e6aa56ea58a5bc79dee8d441645eb4d6e Mon Sep 17 00:00:00 2001 From: Michael Diarmid Date: Sat, 1 Sep 2018 19:13:14 +0100 Subject: [PATCH 07/10] [android] update ShareModuleTest SimplePromise Updated to match `PromiseImpl` changes. --- .../react/modules/share/ShareModuleTest.java | 133 +++++++++++++----- 1 file changed, 97 insertions(+), 36 deletions(-) diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/share/ShareModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/share/ShareModuleTest.java index 86f99cd9dc1928..452eee0e34916a 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/share/ShareModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/share/ShareModuleTest.java @@ -1,4 +1,4 @@ -/** +/* * Copyright (c) 2015-present, Facebook, Inc. * * This source code is licensed under the MIT license found in the @@ -11,31 +11,31 @@ import android.content.Intent; import com.facebook.react.bridge.Arguments; +import com.facebook.react.bridge.JavaOnlyMap; import com.facebook.react.bridge.Promise; -import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactTestHelper; -import com.facebook.react.bridge.JavaOnlyMap; - -import javax.annotation.Nullable; +import com.facebook.react.bridge.WritableMap; import org.junit.After; import org.junit.Before; -import org.junit.Test; import org.junit.Rule; +import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.rule.PowerMockRule; -import org.robolectric.internal.ShadowExtractor; -import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.internal.ShadowExtractor; import org.robolectric.shadows.ShadowApplication; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -53,12 +53,14 @@ public class ShareModuleTest { @Before public void prepareModules() throws Exception { PowerMockito.mockStatic(Arguments.class); - Mockito.when(Arguments.createMap()).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - return new JavaOnlyMap(); - } - }); + Mockito + .when(Arguments.createMap()) + .thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + return new JavaOnlyMap(); + } + }); mShareModule = new ShareModule(ReactTestHelper.createCatalystContextForTest()); } @@ -83,17 +85,34 @@ public void testShareDialog() { mShareModule.share(content, dialogTitle, promise); - final Intent chooserIntent = - ((ShadowApplication)ShadowExtractor.extract(RuntimeEnvironment.application)).getNextStartedActivity(); + final Intent chooserIntent = + ((ShadowApplication) ShadowExtractor.extract(RuntimeEnvironment.application)).getNextStartedActivity(); assertNotNull("Dialog was not displayed", chooserIntent); assertEquals(Intent.ACTION_CHOOSER, chooserIntent.getAction()); - assertEquals(dialogTitle, chooserIntent.getExtras().get(Intent.EXTRA_TITLE)); - - final Intent contentIntent = (Intent)chooserIntent.getExtras().get(Intent.EXTRA_INTENT); + assertEquals( + dialogTitle, + chooserIntent + .getExtras() + .get(Intent.EXTRA_TITLE) + ); + + final Intent contentIntent = (Intent) chooserIntent + .getExtras() + .get(Intent.EXTRA_INTENT); assertNotNull("Intent was not built correctly", contentIntent); assertEquals(Intent.ACTION_SEND, contentIntent.getAction()); - assertEquals(title, contentIntent.getExtras().get(Intent.EXTRA_SUBJECT)); - assertEquals(message, contentIntent.getExtras().get(Intent.EXTRA_TEXT)); + assertEquals( + title, + contentIntent + .getExtras() + .get(Intent.EXTRA_SUBJECT) + ); + assertEquals( + message, + contentIntent + .getExtras() + .get(Intent.EXTRA_TEXT) + ); assertEquals(1, promise.getResolved()); } @@ -111,7 +130,8 @@ public void testInvalidContent() { } final static class SimplePromise implements Promise { - private static final String DEFAULT_ERROR = "EUNSPECIFIED"; + private static final String ERROR_DEFAULT_CODE = "EUNSPECIFIED"; + private static final String ERROR_DEFAULT_MESSAGE = "Error not specified."; private int mResolved; private int mRejected; @@ -126,7 +146,7 @@ public int getResolved() { public int getRejected() { return mRejected; } - + public Object getValue() { return mValue; } @@ -147,31 +167,72 @@ public void resolve(Object value) { @Override public void reject(String code, String message) { - reject(code, message, /*Throwable*/null); + reject(code, message, /*Throwable*/null, /*WritableMap*/null); } @Override - @Deprecated - public void reject(String message) { - reject(DEFAULT_ERROR, message, /*Throwable*/null); + public void reject(String code, Throwable throwable) { + reject(code, /*Message*/null, throwable, /*WritableMap*/null); + } + + @Override + public void reject(String code, String message, Throwable throwable) { + reject(code, message, throwable, /*WritableMap*/null); } @Override - public void reject(String code, Throwable e) { - reject(code, e.getMessage(), e); + public void reject(Throwable throwable) { + reject(/*Code*/null, /*Message*/null, throwable, /*WritableMap*/null); } @Override - public void reject(Throwable e) { - reject(DEFAULT_ERROR, e.getMessage(), e); + public void reject(Throwable throwable, WritableMap userInfo) { + reject(/*Code*/null, /*Message*/null, throwable, userInfo); } @Override - public void reject(String code, String message, @Nullable Throwable e) { + public void reject(String code, @Nonnull WritableMap userInfo) { + reject(code, /*Message*/null, /*Throwable*/null, userInfo); + } + + @Override + public void reject(String code, Throwable throwable, WritableMap userInfo) { + reject(code, /*Message*/null, throwable, userInfo); + } + + @Override + public void reject(String code, String message, @Nonnull WritableMap userInfo) { + reject(code, message, /*Throwable*/null, userInfo); + } + + @Override + public void reject( + String code, + String message, + @Nullable Throwable throwable, + @Nullable WritableMap userInfo + ) { mRejected++; - mErrorCode = code; - mErrorMessage = message; + + if (code == null) { + mErrorCode = ERROR_DEFAULT_CODE; + } else { + mErrorCode = code; + } + + if (message != null) { + mErrorMessage = message; + } else if (throwable != null) { + mErrorMessage = throwable.getMessage(); + } else { + mErrorMessage = ERROR_DEFAULT_MESSAGE; + } } - } + @Override + @Deprecated + public void reject(String message) { + reject(/*Code*/null, message, /*Throwable*/null, /*WritableMap*/null); + } + } } From 71ed1963faf5027954954a0042511bc6c8d33c43 Mon Sep 17 00:00:00 2001 From: Salakar Date: Fri, 14 Sep 2018 14:59:45 +0100 Subject: [PATCH 08/10] [android][PromiseImpl] add stackTrace length check to ensure we don't cause an `IndexOutOfBounds` exception --- .../src/main/java/com/facebook/react/bridge/PromiseImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java index a589478e55402a..b76fc6609c272e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java @@ -208,7 +208,7 @@ public void reject( // Build an an Array of StackFrames to match JavaScript: // iOS: /Libraries/Core/Devtools/parseErrorStack.js -> StackFrame - for (int i = 0; i < ERROR_STACK_FRAME_LIMIT; i++) { + for (int i = 0; i < stackTrace.length && i < ERROR_STACK_FRAME_LIMIT; i++) { StackTraceElement frame = stackTrace[i]; WritableMap frameMap = new WritableNativeMap(); // NOTE: no column number exists StackTraceElement From 704ee97e39b2769acc3cdca88d7c64f756d362da Mon Sep 17 00:00:00 2001 From: Christoph Nakazawa Date: Tue, 11 Dec 2018 11:28:41 +0900 Subject: [PATCH 09/10] Fix formatting --- .../src/main/java/com/facebook/react/bridge/PromiseImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java index b76fc6609c272e..fd1b71f50eafd9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java @@ -170,7 +170,10 @@ public void reject( @Nullable Throwable throwable, @Nullable WritableMap userInfo ) { - if (mReject == null) return; + if (mReject == null) { + return; + } + WritableNativeMap errorInfo = new WritableNativeMap(); if (code == null) { From cf4170c0d30dd3620a0b14146bf6391132c923eb Mon Sep 17 00:00:00 2001 From: Christoph Nakazawa Date: Tue, 11 Dec 2018 11:51:22 +0900 Subject: [PATCH 10/10] Update PromiseImpl.java --- .../src/main/java/com/facebook/react/bridge/PromiseImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java index 366af658a84921..4c67957eda7e33 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/PromiseImpl.java @@ -173,6 +173,7 @@ public void reject( @Nullable WritableMap userInfo ) { if (mReject == null) { + mResolve = null; return; }