unity-wakatime
unity-wakatime copied to clipboard
vladfaust
HTTP responses
You can review existing code while we are waiting for https://github.com/wakatime/wakatime/issues/181
I'm sorry for the delay, @Hermesiss, I'm a bit busy with messy containers security!
First of all, thanks for your time. I do appreciate your help on the project.
I guess this PR is aimed to suppress malformed Wakatime responses. That's good, I think, as it's a third-party bug and we don't want to disrupt the user experience.
A small note — it's generally bad practice to include commented out blocks of code into the master branch. We always have the history, so we could add it back once it's working as intended.
I think the HandleHttpResponseSuccess method is too burden, isn't it enough to check response.error != null and response.status in (200..300) range? And if it's out of that range, just print Got 4xx error...
I may be in some way out of understanding the real purpose of this PR. Which issue particularly does it fix? As per #13, the problem was malformed response, which is now solved.
In everyday programming (especially for such small projects as this) we should rely on third party API to work as expected most of the time, handling all errors in bulk with minimum code. This reduces the complexity and therefore improves maintainability.
In this case, expected error is 429 only, all other are unexpected and should throw. There is no need in this HandleHttpResponseSuccess method, in my opinion.
Additionally, for better communication, you should split the PRs as much as possible, as it's sometimes hard to follow their purpose. Single PR = single change (or complex overhaul).
There are 3 important for us error codes that require special reactions: 0, 429 and 500.
While working on 429 I tried to temporally increase HEARTBEAT_COOLDOWN. I found out that the part where it's used is broken due to some undocumented API changes.
In this PR I want to move scattered data checks to one place. I took all possible codes from dev section.