Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Improve PPA/SafetyNet timeout handling (EXPOSUREAPP-5286) #2436

Merged
merged 6 commits into from
Feb 22, 2021
Merged

Improve PPA/SafetyNet timeout handling (EXPOSUREAPP-5286) #2436

merged 6 commits into from
Feb 22, 2021

Conversation

d4rken
Copy link
Member

@d4rken d4rken commented Feb 22, 2021 •
edited

By @kolyaopahle and me

  • Fix timeout handling within SafetyNet not working due to suspending coroutine not being cancellable
  • Increase SafeyNet timeout to allow API consumers (Survey/Analytics) more freedom in their handling (Analytics can wait much longer) and logs have shown that SafetyNet timeouts can be between 10 and 90+ seconds.
  • We additionally wrap the attestation+submissionToServer call without another timeout check that leaves us enough time so donation modules still have time to cleanup and reach a consistent state. Worker gives us a total window of ~10min, we abort attestation and submission after 360s.
  • Analytics now returns a Result(success: Boolean, retry: Boolean) and depending on that result the DataDonationAnalyticsPeriodicWorker will retry again or just execute again on the next day. The current code only goes for retry on timeouts and network errors.

@d4rken d4rken added bug Something isn't working maintainers Tag pull requests created by maintainers prio PRs to review first. labels Feb 22, 2021
@d4rken d4rken added this to the 1.13.0 milestone Feb 22, 2021
@d4rken d4rken requested a review from a team February 22, 2021 16:46
@harambasicluka harambasicluka mentioned this pull request Feb 22, 2021
Merged
@d4rken
Tweak comment & log message.
e9e4a12
@d4rken
Merge remote-tracking branch 'origin/fix/5286-ppa-analytics-timeoutha…
a84d345 
…ndling' into fix/5286-ppa-analytics-timeouthandling
@d4rken
Merge branch 'release/1.13.x' into fix/5286-ppa-analytics-timeouthand…
c4ced7d 
…ling

# Conflicts:
#	Corona-Warn-App/src/main/java/de/rki/coronawarnapp/datadonation/analytics/worker/DataDonationAnalyticsScheduler.kt
@ralfgehrer ralfgehrer self-assigned this Feb 22, 2021
ralfgehrer
ralfgehrer approved these changes Feb 22, 2021
View reviewed changes
Copy link
Contributor

@ralfgehrer ralfgehrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@sonarcloud
Copy link

sonarcloud bot commented Feb 22, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.9% 87.9% Coverage
0.0% 0.0% Duplication

@ralfgehrer ralfgehrer merged commit 2c67668 into release/1.13.x Feb 22, 2021
@ralfgehrer ralfgehrer deleted the fix/5286-ppa-analytics-timeouthandling branch February 22, 2021 17:13
kolyaopahle
kolyaopahle approved these changes Feb 22, 2021
View reviewed changes
Copy link
Contributor

@kolyaopahle kolyaopahle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ralfgehrer ralfgehrer ralfgehrer approved these changes

@kolyaopahle kolyaopahle kolyaopahle approved these changes

@chris-cwa chris-cwa chris-cwa approved these changes

Assignees

@ralfgehrer ralfgehrer

Labels
bug Something isn't working maintainers Tag pull requests created by maintainers prio PRs to review first.
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@d4rken @ralfgehrer @kolyaopahle @chris-cwa @harambasicluka