Attempt to fix iOS error where no response is available.

daveware-nv commented 5 years ago

PR Checklist

[ ] The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
[x] There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
[ ] All existing tests are passing
[ ] Tests for the changes are included

What is the current behavior?

On iOS the response is always null and responseCode always -1. So we can't tell if the upload was successful.

What is the new behavior?

I've added a parameter when creating a session on iOS to cause it to use the default session instead of a background session.

export function session(id: string, foreground?: boolean)

When setting this new option to true, the response will be passed through as expected. With the caveat the the session will not operate in background mode.

Further details of my changes and rationale can be found in the commit message.

Creates workaround for #214 .

BREAKING CHANGES:

Using the foreground option on iOS will cause the session to operate in the default mode, rather than background. Meaning it is not a solution for people specifically looking for background uploads.
I've also reorganized when the "responded" event fires to line up with the android behavior. I don't think this is likely to cause problems.

cla-bot[bot] commented 5 years ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. CLA has not been signed by users: @daveware-nv. After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

tbozhikov commented 5 years ago

Hey @daveware-nv, thanks a lot for your contribution! 🥇 The issue you are trying to fix here turned out to be buried deeper in the {N} iOS Runtime 5.3+ due to a bug introduced there. So, a revamp of the native properties read logic had to be implemented in https://github.com/NativeScript/nativescript-background-http/pull/222. With this, we can close this PR as it addresses the same bug in a different way.

NativeScript / nativescript-background-http