Wrong status code crashes app
tapir opened this issue · 5 comments
Today api.telegram.org gives "502 bad gateway error" or "504 gateway timeout" error sometimes. Somehow response.status is 1 so the error page is sent to JSON decoding and everything comes down burning. I'm not sure if it's a luasec problem or it's used wrongly in this library but I've managed to solve it buy changing this line
success = success or false
to this line
success = tonumber(code) < 400
and all the
response.success == 1
to
response.success
PS: I didn't create a pull request for this because I don't know if this is luasec's fault or not.
Edit: Fixed the solution
Hello,
you are right, 502 bad gateway error
or 504 gateway timeout
are not caught.
The way it works is that
response.success
returns 1
if the actual request was successful, and nil
if not.
This does not include the actual HTTP response code, which is returned in response.code
.
The reason I don't check for the actual response code, is because other codes such as
401 Unauthorized
still return valid JSON, which can (and should) be parsed, to make debugging of API calls easier.
The problem is that the Server does not return (valid) JSON on the errors above.
So, instead of mixing status and response variables, which might leave errors such as 401
flagged as a false positive, a better approach would be to catch errors produced while decoding JSON.
I will include functions to handle bad JSON errors.
Also, I may include some sort of logging.
If you have records of the errors you received, you can post them here, so I can have a look.
I'm not very informed about HTTP status codes but shouldn't, at least, every 5xx error cause a "status.success = false". There is practically no difference between an unsuccessful HTTP request and a server error, as far as a telegram bot is concerned.
Also how does one get 401 from api.telegram.org?
If you want to handle it that way, you can.
However other people might want to react differently to those errors, and I would feel bad for taking this opportunity from them, since I want to leave as many possibilities open as possible.
Please check out the latest commit in the dev branch (@a512957).
It adds JSON Error handlers which do not assert on en-/decoding, thus should no longer crash the bot.
You can get error 401 Unauthorized
from the API by providing a wrong token.
Fair enough. But then below statement and code are in contradiction.
response.success returns 1 if the actual request was successful, and nil if not.
This does not include the actual HTTP response code, which is returned in response.code.
if (response.success == 1) then
return JSON:decode(response.body)
else
return nil, response.code
end
If luasec status is not related to the HTTP status code why are we returning it when there is an unrelated error?
Good observation, this indeed does not make much sense.
I will put in a more descriptive error message later.
Also, upon error, response.success
does not become nil, as previously stated, but "false"
.
I presume the issue you had should be closed by now, please try it out.