Request reattempt fix #418
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
• Our SDK recently introduced HTTP reattempts for failed (500+) type requests. • However, our SDK also had a timeout where it would give up on an HTTP request if it took too long • This became apparent when calls to sendTags() when users had no network connection would, after 60 seconds, call the successBlock • The success block was being called because the final re-attempt had not yet occurred, so no errors had been populated in the errors dictionary • Fixed by (A) fixing the timeout to give enough time for all re-attempts to finish and (B) ensuring that there will be an error added to the errors dictionary if a request times out.
• Adds testing to ensure that the reattempt logic works correctly
jkasten2
approved these changes
Sep 21, 2018
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved
|
Due to time constraints we will be merging this PR. The added test is flaky and its failure is only reproducible on Travis. |
Nightsd01
added a commit
that referenced
this pull request
Sep 25, 2018
This reverts commit 937b44f.
Nightsd01
added a commit
that referenced
this pull request
Sep 25, 2018
This reverts commit 937b44f.
Nightsd01
added a commit
that referenced
this pull request
Sep 25, 2018
* revert-412-dynamic_framework: Revert "Dynamic framework (#412)" Revert "2.8.8 Release" Revert "Request reattempt fix (#418)" # Conflicts: # OneSignal.podspec # OneSignalDynamic.podspec # iOS_SDK/OneSignalSDK/Framework/Dynamic/OneSignal.framework/OneSignal # iOS_SDK/OneSignalSDK/Source/OneSignal.m # iOS_SDK/OneSignalSDK/UnitTests/UnitTests.m NOTE: Mistakes during a merge from branch `dynamic_framework` to `master` led to some previous bug fixes being reverted. • Using a partial revert merge to undo these reversions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
• Fixes an issue where network requests were not being allowed to finish all allocated reattempts
• Fixes an issue where calling
sendTags()(and other requests) with no active internet connection would result in thesuccessblock eventually getting called even though the request did not succeed• Adds regression testing to verify reattempt logic
This change is