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.

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

[IOS][BUGFIX] [DynamicLinks] Retry resolving the dynamic link once when the error is an ECONNABORTED #1841

Merged
merged 1 commit into from
Jan 18, 2019

Conversation

benschell
Copy link
Contributor

@benschell benschell commented Jan 17, 2019

Summary

I ran into the issue described in #1589 ; specifically, a ECONNABORTED when returning to my app (while the app was in the background) from a click on a dynamic link (in my case, it was specifically a password-less email login link).

The comments in that issue pointed to some suggested workarounds present in a discussion on AFNetworking; I specifically read through this comment https://github.com/AFNetworking/AFNetworking/issues/4279#issuecomment-447108981 and implemented suggested workaround (B) which was to, in the case of this specific error condition, retry the request.

I'm not sure if this should be against a point-release branch instead of master, please forgive me if it's pointing to the wrong branch.

Checklist

  • [N/A] Supports Android
  • [Y] Supports iOS
  • e2e tests added or updated in /tests/e2e/*
  • [N/A] Updated the documentation in the docs repo
  • [N/A] Flow types updated
  • [N/A] Typescript types updated

Test Plan

Release Plan

[IOS][BUGFIX] [DynamicLinks] - Fixed a bug where dynamic links were not handled properly on iOS when returning from a background state

🔥

@CLAassistant
Copy link

CLAassistant commented Jan 17, 2019

CLA assistant check
All committers have signed the CLA.

@benschell benschell changed the title Retry resolving the dynamic link once when the error is an ECONNABORTED [IOS][BUGFIX] [DynamicLinks] Retry resolving the dynamic link once when the error is an ECONNABORTED Jan 17, 2019
@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #1841 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1841   +/-   ##
=======================================
  Coverage   78.22%   78.22%           
=======================================
  Files          63       63           
  Lines        1510     1510           
=======================================
  Hits         1181     1181           
  Misses        329      329

3 similar comments
@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #1841 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1841   +/-   ##
=======================================
  Coverage   78.22%   78.22%           
=======================================
  Files          63       63           
  Lines        1510     1510           
=======================================
  Hits         1181     1181           
  Misses        329      329

@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #1841 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1841   +/-   ##
=======================================
  Coverage   78.22%   78.22%           
=======================================
  Files          63       63           
  Lines        1510     1510           
=======================================
  Hits         1181     1181           
  Misses        329      329

@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #1841 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1841   +/-   ##
=======================================
  Coverage   78.22%   78.22%           
=======================================
  Files          63       63           
  Lines        1510     1510           
=======================================
  Hits         1181     1181           
  Misses        329      329

@Salakar
Copy link
Member

Salakar commented Jan 18, 2019

@benschell thank you for this, all is good here and master is fine 👌 will go ahead and merge for the v5.3.x pipeline.

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.

3 participants