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

Consolidated Error Handling #20

Merged
merged 1 commit into from
Mar 21, 2022
Merged

Consolidated Error Handling #20

merged 1 commit into from
Mar 21, 2022

Conversation

richardpiazza
Copy link
Contributor

Consolidates all of the errors produced by the client into a single enum. Although not entirely needed, the majority of errors that could be potentially thrown were from APIManager.NetworkError. With that being an internal type, that limited the ability of a client consumer from being able to type catch and potentially resolve issues. All errors are now of type FlagsmithError.

Additional notes:

  • Moved Router to its own file and added the ability to pass a JSONEncoder to the request method enabling unit tests.
  • Refactored the APIManager request method to not need the emptyResponse parameter. The conditional checking for a specific type and flag was a bit cumbersome to follow. Now the completion block dictates if the response is decoded vs just indicating success/failure.
  • Replaced the fatalError() in the APIManager with an error completion. Questionable wether the library should cause an application to crash for being misconfigured.
  • Added additional documentation comments to fill out automatic DocC documentation that can be generated in Xcode.
  • Unit tests for Router and APIManager where possible.

I know I can go a bit overboard with some changes and opinionated behaviors. Please, feel free to reject or request changes. The company I work for is likely adopting Flagsmith, hence all the PRs!

@dabeeeenster dabeeeenster merged commit 3e966ee into Flagsmith:main Mar 21, 2022
@dabeeeenster
Copy link
Collaborator

This is great - thanks so much

@richardpiazza richardpiazza deleted the feature/error-handling branch March 21, 2022 16:48
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.

2 participants