Skip to content

Couldn't parse org rules#420

Merged
tomholub merged 5 commits intomasterfrom
feature/issue-408-couldnt-parse-organisational-rules-calling-fes
Jul 30, 2021
Merged

Couldn't parse org rules#420
tomholub merged 5 commits intomasterfrom
feature/issue-408-couldnt-parse-organisational-rules-calling-fes

Conversation

@ekievsky
Copy link
Contributor

@ekievsky ekievsky commented Jul 29, 2021

This PR tolerates not found, timeout, service unavailable errors in EnterpriseService;

close #408


Tests:

  • Does not need tests

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

-> did I receive a response and it's other status 4xx or 5xx? -> show error to user

Where was this implemented?

-> did I NOT receive a response (server down or timeout) and the internet is otherwise working? -> don't use FES, empty OrgRules

Where was this implemented?

-> did I NOT receive a response (server down or timeout) and the internet is not working? -> show error to user

Where was this implemented?


This PR only addresses 20% of the requirements that were clearly laid out in the issue. If the PR addresses less then all requirements of an issue, you must clearly highlight which parts you addressed, and don't close the issue in the PR description.

@tomholub tomholub marked this pull request as draft July 29, 2021 11:57
@ekievsky ekievsky marked this pull request as ready for review July 29, 2021 14:39
Comment on lines +36 to +39
static let getToleratedHTTPStatuses = [404]
/// -1001 - request timed out, -1003 - сannot resolve host
static let getToleratedNSErrorCodes = [-1001, -1003]
static let getActiveFesTimeout: TimeInterval = 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should also tolerate the following, which all are related to server down

  kCFURLErrorCannotConnectToHost    = -1004,
  kCFURLErrorNetworkConnectionLost  = -1005,
  kCFURLErrorDNSLookupFailed        = -1006,
  kCFURLErrorHTTPTooManyRedirects   = -1007,
  kCFURLErrorResourceUnavailable    = -1008,

static let getToleratedHTTPStatuses = [404]
/// -1001 - request timed out, -1003 - сannot resolve host
static let getToleratedNSErrorCodes = [-1001, -1003]
static let getActiveFesTimeout: TimeInterval = 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, what is the number 100 here, 100 seconds? Milliseconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for some reason my lates commit wasn't included :/

@tomholub tomholub enabled auto-merge (squash) July 30, 2021 17:12
@tomholub tomholub merged commit 791ae51 into master Jul 30, 2021
@tomholub tomholub deleted the feature/issue-408-couldnt-parse-organisational-rules-calling-fes branch July 30, 2021 17:26
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.

couldn't parse data while fetching organizational rules

2 participants