Add missing catch

[robinsoncol]

No description provided.

[request-info]

We would appreciate it if you could provide us with more info about this issue/pr! :smiley

[matheusmichels]

This is a pretty important fix, since the Share.open() promise will never get resolved if the native promise is rejected
@jgcmarins @MateusAndrade @mikehardy

[MateusAndrade]

Details

Hi, it looks like there are some linting errors: https://app.circleci.com/pipelines/github/react-native-share/react-native-share/1095/workflows/caa224b0-23d0-4c2c-a2b5-75058e9e03b2/jobs/4204

[mikehardy]

This is pretty difficult to follow (not your fault!) because of the nesting, but I carefully went through and counted open/close parentheses and this is right where the native open/then structure could have a catch, and there is no catch

What I don't understand:

Prior to this PR the native could throw without an immediate catch, meaning it should bubble up to the enclosing context, which is a then on requireAndAskPermissions

According to this info about exceptions thrown while in a then block, https://stackoverflow.com/a/38690864/9910298 - that should go to the next catch.

So I don't understand why the enclosing catch doesn't already function the same as what you have?

That said, the catch you have added is doing the same thing the enclosing one is doing, and is not capable of doing anything wrong as there is no logic between the new catch and the existing final catch that will be missed or similar, so I cannot see how this would harm, and you indicate this fixes a failure to resolve for you

So I'm +1 on it, with the final criteria being: you have tested this and it fixes something for you :-)

Final thought: should the reject's be return reject, similar to the return resolve ?

[robinsoncol]

@mikehardy @MateusAndrade @jgcmarins The error isn't propagating because the promise returned by NativeRNShare.open(...).then(...) isn't being returned. If you return NativeRNShare.open(...).then(...) you won't need the fix above, but both solutions solve the same issue.

There seems to be a lot unnecessary promise nesting. It's best to merge this PR to fix this issue and I'll submit another PR to clean up the code.

[robinsoncol]

Details

Hi, it looks like there are some linting errors: https://app.circleci.com/pipelines/github/react-native-share/react-native-share/1095/workflows/caa224b0-23d0-4c2c-a2b5-75058e9e03b2/jobs/4204

@MateusAndrade This should be fixed now

[mikehardy]

I'm +1 on this, for what it's worth, and I appreciate the explanations + discussion, thanks!

[MateusAndrade]

There is an iOS build error, but it's not related to this PR. So, it should be ok to merge it. Thanks @robinsoncol !