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.

Sign up for GitHub

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

Jump to bottom

feat(Promise): Adding userInfo to reject error object #732

Merged
merged 2 commits into from
Oct 5, 2016
Merged

feat(Promise): Adding userInfo to reject error object #732

merged 2 commits into from
Oct 5, 2016

Conversation

rozele
Copy link
Collaborator

@rozele rozele commented Oct 3, 2016

Using System.Exception.Data IDictionary, users can add arbitrary data to a "userInfo" property on the error object.

Fixes #730

henningms
henningms approved these changes Oct 4, 2016
View reviewed changes
Copy link

@henningms henningms left a comment

Choose a reason for hiding this comment

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

This looks good. Will certainly allow us to embed objects that we can pass along without hassle.

@rozele
feat(Promise): Adding userInfo to reject error object
72f477c 
Using System.Exception.Data IDictionary, users can add arbitrary data to a "userInfo" property on the error object.

Fixes microsoft#730
rozele
rozele commented Oct 5, 2016
View reviewed changes
ReactWindows/ReactNative.Shared/Bridge/Promise.cs
{ "message", message },
{ "stack", e?.StackTrace },
{ "userInfo", userInfo },
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix whitespace

@rozele rozele merged commit 4f089d8 into microsoft:master Oct 5, 2016
@rozele rozele deleted the issue730 branch October 5, 2016 15:27
This was referenced Aug 31, 2018
Closed
Closed
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Dec 11, 2018
@Salakar @facebook-github-bot
Allow 'userInfo' for native promise.reject + add native error stack s…
794d226 
…upport (#20940)

Summary:
As mentioned [here](react-native-community/releases#34 (comment)), Android is missing native Promise reject with a `userInfo` `WritableMap` support and also `nativeStack` support (which addresses `TODO(8850038)`). This PR adds Android support for both of these.

React Native on iOS ([here](https://github.com/facebook/react-native/blob/master/React/Base/RCTUtils.m#L433)) and Windows ([here](microsoft/react-native-windows#732)) already support this so this is a relatively minor addition to bring Android in line with the other platforms. (JS support is also [here](https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/NativeModules.js#L145-L148))

Existing methods remain unchanged other than general cleanup of variable names (`e -> throwable`) and adding code comments/docs.

Additionally, the `ShareTestModule` implementation of Promise (SimplePromise) was updated to reflect these changes - other line changes in this file are from formatting in Android Studio - if this is an issue let me know.

 - Currently inconsistent with other platforms.
 - Blocking a couple of issues over at [invertase/react-native-firebase](https://github.com/invertase/react-native-firebase) - save for re-writing everything to Promise resolve only - which is no small change and isn't a great solution either.
Pull Request resolved: #20940

Differential Revision: D13412527

Pulled By: cpojer

fbshipit-source-id: 2ca6c5f3db9ff2c2986b02edda80bc73432f66d3
grabbou pushed a commit to facebook/react-native that referenced this pull request Dec 17, 2018
@Salakar @grabbou
Allow 'userInfo' for native promise.reject + add native error stack s…
1d949a1 
…upport (#20940)

Summary:
As mentioned [here](react-native-community/releases#34 (comment)), Android is missing native Promise reject with a `userInfo` `WritableMap` support and also `nativeStack` support (which addresses `TODO(8850038)`). This PR adds Android support for both of these.

React Native on iOS ([here](https://github.com/facebook/react-native/blob/master/React/Base/RCTUtils.m#L433)) and Windows ([here](microsoft/react-native-windows#732)) already support this so this is a relatively minor addition to bring Android in line with the other platforms. (JS support is also [here](https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/NativeModules.js#L145-L148))

Existing methods remain unchanged other than general cleanup of variable names (`e -> throwable`) and adding code comments/docs.

Additionally, the `ShareTestModule` implementation of Promise (SimplePromise) was updated to reflect these changes - other line changes in this file are from formatting in Android Studio - if this is an issue let me know.

 - Currently inconsistent with other platforms.
 - Blocking a couple of issues over at [invertase/react-native-firebase](https://github.com/invertase/react-native-firebase) - save for re-writing everything to Promise resolve only - which is no small change and isn't a great solution either.
Pull Request resolved: #20940

Differential Revision: D13412527

Pulled By: cpojer

fbshipit-source-id: 2ca6c5f3db9ff2c2986b02edda80bc73432f66d3
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 2019
@Salakar @facebook-github-bot
Allow 'userInfo' for native promise.reject + add native error stack s…
2ef7bfc 
…upport (facebook#20940)

Summary:
As mentioned [here](react-native-community/releases#34 (comment)), Android is missing native Promise reject with a `userInfo` `WritableMap` support and also `nativeStack` support (which addresses `TODO(8850038)`). This PR adds Android support for both of these.

React Native on iOS ([here](https://github.com/facebook/react-native/blob/master/React/Base/RCTUtils.m#L433)) and Windows ([here](microsoft/react-native-windows#732)) already support this so this is a relatively minor addition to bring Android in line with the other platforms. (JS support is also [here](https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/NativeModules.js#L145-L148))

Existing methods remain unchanged other than general cleanup of variable names (`e -> throwable`) and adding code comments/docs.

Additionally, the `ShareTestModule` implementation of Promise (SimplePromise) was updated to reflect these changes - other line changes in this file are from formatting in Android Studio - if this is an issue let me know.

 - Currently inconsistent with other platforms.
 - Blocking a couple of issues over at [invertase/react-native-firebase](https://github.com/invertase/react-native-firebase) - save for re-writing everything to Promise resolve only - which is no small change and isn't a great solution either.
Pull Request resolved: facebook#20940

Differential Revision: D13412527

Pulled By: cpojer

fbshipit-source-id: 2ca6c5f3db9ff2c2986b02edda80bc73432f66d3
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
@Salakar @grabbou
Allow 'userInfo' for native promise.reject + add native error stack s…
8420e5d 
…upport (facebook#20940)

Summary:
As mentioned [here](react-native-community/releases#34 (comment)), Android is missing native Promise reject with a `userInfo` `WritableMap` support and also `nativeStack` support (which addresses `TODO(8850038)`). This PR adds Android support for both of these.

React Native on iOS ([here](https://github.com/facebook/react-native/blob/master/React/Base/RCTUtils.m#L433)) and Windows ([here](microsoft/react-native-windows#732)) already support this so this is a relatively minor addition to bring Android in line with the other platforms. (JS support is also [here](https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/NativeModules.js#L145-L148))

Existing methods remain unchanged other than general cleanup of variable names (`e -> throwable`) and adding code comments/docs.

Additionally, the `ShareTestModule` implementation of Promise (SimplePromise) was updated to reflect these changes - other line changes in this file are from formatting in Android Studio - if this is an issue let me know.

 - Currently inconsistent with other platforms.
 - Blocking a couple of issues over at [invertase/react-native-firebase](https://github.com/invertase/react-native-firebase) - save for re-writing everything to Promise resolve only - which is no small change and isn't a great solution either.
Pull Request resolved: facebook#20940

Differential Revision: D13412527

Pulled By: cpojer

fbshipit-source-id: 2ca6c5f3db9ff2c2986b02edda80bc73432f66d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henningms henningms henningms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rozele @henningms