Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@halseth
Copy link
Contributor

@halseth halseth commented Aug 7, 2019

It will also be part of the passed NSError, but this makes the error
message explicit.

It will also be part of the passed NSError, but this makes the error
message explicit.
@valentinewallace valentinewallace self-requested a review August 7, 2019 16:40
Copy link
Contributor

@valentinewallace valentinewallace left a comment

Choose a reason for hiding this comment

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

LGTM 🥇 Ty @halseth!! I pushed a follow-up commit to display the error rather than err.details, tested and it worked for me.

} catch (err) {
this._notification.display({
msg: `Fee estimation failed: ${err.details}`,
msg: `Fee estimation failed: ${err}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

@halseth can we change it so that err is an object that has the string on a message attribute so that we can access it like a normal JS Error like so: err.message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't really set anything, it is react-native that creates this error object.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, we're not getting an error object here. err is a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I guess that's what React Native throws then :)

@tanx
Copy link
Contributor

tanx commented Aug 12, 2019

Closing in favor of #1292

@tanx tanx closed this Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants