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.

Sign up for GitHub

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

Jump to bottom

limited scope of of retry logic to very specific 500-level codes from… #165

Merged
merged 2 commits into from
Nov 20, 2023
Merged

limited scope of of retry logic to very specific 500-level codes from… #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c0cbf1
limited scope of of retry logic to very specific 500-level codes from…
lugehorsam Nov 17, 2023
fd9aacf
remove 500
lugehorsam Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
## [Unreleased]
### Changed
- Nakama: Mark socket as connected before event handler is called.
- Nakama: Limited scope of of retry logic to very specific 500-level codes from the server.

### Added
- Nakama: Rank count is now returned with tournament record listings.
Expand Down
13 changes: 12 additions & 1 deletion Nakama/HttpRequestAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,18 @@ public static IHttpAdapter WithGzip(bool decompression = false, bool compression

private bool IsTransientException(Exception e)
{
return (e is ApiResponseException apiException && (apiException.StatusCode >= 500 || apiException.StatusCode == -1)) || e is HttpRequestException;
if (e is ApiResponseException apiException)
{
switch (apiException.StatusCode)
{
case 502:
case 503:
case 504:
return true;
}
}

return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we intentionally removing -1 from the list of monitored exception status codes here?
Also, I would tend to agree that we probably don't want to retry on an Internal Server Error as it's likely that more often than not the same error will occur if nothing about the request has changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's intentional. Originally I think I spoke with @novabyte about checking for that value if we don't receive a reply from the load balancer, but looking at the client code, the client timeout won't create an ApiResponseException.

}
}
}
Loading