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

error handling #86

Merged
merged 4 commits into from
Apr 24, 2022
Merged

Conversation

phenpessoa
Copy link
Contributor

@phenpessoa phenpessoa commented Mar 2, 2022

Ok, this is a massive PR and also a rewrite of good chunks of the code. I splitted it into a few commits to make it easier to follow. I'll also try to summarize it as much as I can here.

Reasoning: the webserver was already running on prod and was panicking on pretty much every error, which is not good.
Besides that, invalid inputs (for example: 123456 as a char name) would return an "empty" json instead of an error.

Changes:

1: Static package

I created a static package and moved all test files inside it (without any changes to the test files). The reason was to be able to embed the files. Since I created "nested" packages, this helps a lot with dealing with file paths.

2: Tibia mapping package

Created a separate package to fetch data from assets.tibiadata.com. This was mainly to organize the code.

3: Validation package

This is one of the biggest changes. This is where all the validation is happening. I even moved some validation that was being handled in the main package into here.

This is how this package works: I implemented a bunch of error codes (errors.go), this makes it easier to debug code and also give users a more reasonable error response when making a request.
I then made all handlers funcs validate their input (when needed), this reduces the amount of API calls made to tibia.com, but, most importantly, it gives us great control as to what error to return to the user. For example: instead of only responding with "character not found" we can give the user an error that exists in the input (name too small or too big for example).
This package also has the limits.go file which are constants from tibia.com

4: Env Vars funcs logic fix

This is a small change, isEnvExist and getEnv were simplified and had their logics fixed. (#91)

5: Consistency changes

I changed all Tibiadata names to TibiaData, just for consistency. (#90)
Stoped using ioutil as it is deprecated. (#92)

6: Debug endpoint

I created a /debug endpoint that shows some useful info about the validation package.

7: Error Handling

This is the biggest change on this PR. Here's what was done:
All "Impl" funcs now return a pointer to their respective response and an error. This is how go handle errors. Before this, we were panicking on every error. Now we just return the error to the user.

I changed a few ".Each" funcs to ".EachWithBreak", this is to be able to break the loop if an error occurs.

Character not found, creature not found, spell not found, etc are now properly handled.

Added a Status struct to the Information struct, this is returned on every request for consistency and contains these fields: HTTPCode, Error, Message.
Error is the int value of the error, and Message is a readable message from the error.

Created the TibiaDataErrorHandler that should be used everytime we want to return an error.
TibiaDataAPIHandleResponse does not handle errors anymore and should not be called in case of an error.

@tobiasehlert tobiasehlert changed the base branch from main to feature-error-handling March 2, 2022 08:08
@tobiasehlert tobiasehlert added enhancement New feature or request go Pull requests that update Go code WIP Work in progress labels Mar 2, 2022
@tobiasehlert tobiasehlert linked an issue Mar 2, 2022 that may be closed by this pull request
@tobiasehlert tobiasehlert changed the base branch from feature-error-handling to main March 4, 2022 11:05
@tobiasehlert tobiasehlert changed the base branch from main to feature-error-handling March 4, 2022 11:11
@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #86 (ec17dc7) into feature-error-handling (e68b1db) will increase coverage by 1.18%.
The diff coverage is 100.00%.

❗ Current head ec17dc7 differs from pull request most recent head f08fd5d. Consider uploading reports for the commit f08fd5d to get more accurate results

@@                    Coverage Diff                     @@
##           feature-error-handling      #86      +/-   ##
==========================================================
+ Coverage                   79.41%   80.59%   +1.18%     
==========================================================
  Files                          21       21              
  Lines                        2647     2669      +22     
==========================================================
+ Hits                         2102     2151      +49     
+ Misses                        474      451      -23     
+ Partials                       71       67       -4     

@phenpessoa
Copy link
Contributor Author

As discussed in the discord server, some http codes should be updated.
I think this is the way to go. Let me know if you agree @tobiasehlert and I'll update the PR accordingly.

  • 400 if character house, creature, etc was malformed (abc123 as the character name for example)
  • 404 if character, house, creature, etc was not found
  • 503 if tibia is in maintenance mode

@phenpessoa phenpessoa force-pushed the errors branch 2 times, most recently from f08fd5d to af99cc0 Compare April 24, 2022 11:20
@sonarcloud
Copy link

sonarcloud bot commented Apr 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@tobiasehlert tobiasehlert merged commit 348ed9c into TibiaData:feature-error-handling Apr 24, 2022
@tobiasehlert tobiasehlert removed the WIP Work in progress label Apr 24, 2022
tobiasehlert pushed a commit that referenced this pull request Apr 26, 2022
* add tibia mapping
* create the static package
* create the validation package
* implement error handling
tobiasehlert pushed a commit that referenced this pull request Apr 26, 2022
* add tibia mapping
* create the static package
* create the validation package
* implement error handling
tobiasehlert pushed a commit that referenced this pull request May 31, 2022
* add tibia mapping
* create the static package
* create the validation package
* implement error handling
tobiasehlert pushed a commit that referenced this pull request Jun 7, 2022
* add tibia mapping
* create the static package
* create the validation package
* implement error handling
tobiasehlert pushed a commit that referenced this pull request Jan 25, 2023
* add tibia mapping
* create the static package
* create the validation package
* implement error handling
tobiasehlert added a commit that referenced this pull request Jan 27, 2023
* error handling (#86)

* add tibia mapping
* create the static package
* create the validation package
* implement error handling

* adding response codes for failures
even if the codes from Kong are missing too..

* adding codecov proxy support
due to failure of tests being rate-limited

* adjusting log message with URL

* fixing docker build issue
adding modules for every module inside the src folder
adjusting Dockerfile order of copy and build step
updating go mod and workflows to go version 1.18

* removing moved testdata files
due to not being deleted when rebased

* adjusting some more code things

Co-authored-by: Pedro Pessoa <pedro_santos_40@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

Improvement of error responses from the API
2 participants