Skip to content

Attempt to fix iOS error where no response is available. #220

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.

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

Conversation

daveware-nv
Copy link

PR Checklist

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.

- For some reason the ``response`` property on the NSURLSessionDataTask
  is always null.
- The only other place the response seems to be available is delegate method
  ``URLSessionDataTaskDidReceiveResponseCompletionHandler``
  which is not called in background mode, or
  NSURLSession.uploadTaskWithRequestFromFile which bypasses many delegate methods
  and also isn't available in background mode
- To work around this I added a session construction property to start
 in foreground, and then saved the response when it arrived.
@cla-bot
Copy link

cla-bot bot commented May 29, 2019

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
Copy link
Contributor

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 #222. With this, we can close this PR as it addresses the same bug in a different way.

@tbozhikov tbozhikov closed this Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants