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

[Fix]Handle incorrect 404 responses; add a delay after creates and retries on 404 of new ids #2059

Merged
merged 6 commits into from
Apr 22, 2024

Commits on Apr 17, 2024

  1. create test to prove we don't wait after create

    This is to account for the case where we create a User or Subscription
    and attempt to immediately access it (via GET or PATCH). A delay is
    needed as the backend may incorrectly 404 otherwise, due to a small
    delay in it's server replication.
    A cold down period like this also helps improve batching as well.
    jkasten2 committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    83fa82b View commit details
    Browse the repository at this point in the history
  2. Add opRepoPostCreateDelay

    This is a new behavior which wakes a specific amount of time after
    something is created (such as a User or Subscription) before make a
    network call to get or update it.
    
    The main motivation is that the OneSignal's server may return a 404 if
    you attempt to GET or PATCH on something that was just created. This is
    due fact that OneSignal's backend server replication sometimes has a
    delay under load. This may be considered a bug in the backend, but on
    the flip side the SDK is being inefficient if a follow up network
    request is made any way, which was the 2nd motivation for this change.
    jkasten2 committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    59fd1df View commit details
    Browse the repository at this point in the history

Commits on Apr 19, 2024

  1. Retry on 404/410 within short window after create

    In summary, this is fallback logic to opRepoPostCreateDelay. The
    opRepoPostCreateDelay is a balance between not waiting too long
    where the SDK isn't being response, but long enough where the
    likelihood of running into the backend replica delay issue is low.
    
    Given the above, we can further lower the chance of this issue by
    retrying requests that fail with 404 or 410 for Subscriptions.
    We don't want to do this with all 404/410 requests so we utilize the
    same NewRecordsState list, but added a new isInMissingRetryWindow
    method and opRepoPostCreateRetryUpTo value to drive the window value.
    
    Added tests to executors to ensure the new 404 logic is working. Some
    executors were missing 404 handling altogether so we filled in those
    gaps in this commit as well.
    jkasten2 committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    5ffa8a3 View commit details
    Browse the repository at this point in the history
  2. correct applyToRecordId for login operation

    We need to check the existingOnesignalId first as most of the time
    the login executor tries to do an operation to the User, add Alias,
    first instead of creating a new User. This makes sure our
    opRepoPostCreateDelay applies.
    jkasten2 committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    fb10758 View commit details
    Browse the repository at this point in the history
  3. prevent adding executionInterval & postCreateDelay

    After we wait for the opRepoPostCreateDelay time call to waiter.wake()
    would cause waitForNewOperationAndExecutionInterval() to wake up as
    we expect, however it was then waiting for the full time on
    opRepoExecutionInterval as well.
    To solve this we simply subtract the time we already waited from
    opRepoExecutionInterval. With the current values this means we
    fully skip opRepoExecutionInterval so we only wait
    opRepoPostCreateDelay.
    jkasten2 committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    1c2bf82 View commit details
    Browse the repository at this point in the history
  4. only consider retrying on 404, don't include 410

    If the backend returns 410 then it indicates it knows it existed at one
    point and it is deleted now. Where a 404 is more abstract.
    jkasten2 committed Apr 19, 2024
    Configuration menu
    Copy the full SHA
    a9dfc09 View commit details
    Browse the repository at this point in the history