Discordia
Discordia copied to clipboard
Client option to log full reason for HTTP errors
This pull request adds a new boolean
client option logFullErrors
(true
by default) which, when true
, makes the API
class log the full error reason (found inside the response body) of requests that return an error response. This makes debugging much easier as you're able to see exactly what went wrong without having to manually print the error message returned by Discordia methods.
As an example, what before was:
2021-10-23 15:03:39 | [ERROR] | 400 - Bad Request : POST https://discord.com/api/v8/channels/901541671806857238/messages
now becomes:
2021-10-23 15:03:39 | [ERROR] | 400 - Bad Request : POST https://discord.com/api/v8/channels/901541671806857238/messages
HTTP Error 50035 : Invalid Form Body
BASE_TYPE_REQUIRED in payload.embeds[0].fields[0].value : This field is required
Going to wait on this one, but not deny it.
Good enough for 2.x. I can always rewrite it in 3.0. I would rather have this as false
by default though.
It'd be nice for debugging if a file & line number could be added to this, if that's easy to do.
It'd be nice for debugging if a file & line number could be added to this, if that's easy to do.
That is definitely doable with the help of the debug library, making use of getinfo
I would guess, but I don't know if it is worth it. Usually using the debug library has some serious performance implications, and calling this on every request error... that might be a bit too much.
Even if it's disabled by default like Sinister said?
There will always be http errors you cannot avoid. But in the case it was false
by default and be turned on explicitly by the user, and the user was smart about it and didn't turn it on all of the time, it can be useful indeed. Perhaps, if it had its own client option?
Nonetheless, in my opinion this is something we can think of for Discordia 3.
I would rather have this as false by default though.
Done; https://github.com/SinisterRectus/Discordia/pull/319/commits/7d7f1633571da421fb5aa32821b5636008b8716e
It'd be nice for debugging if a file & line number could be added to this, if that's easy to do.
Agreed, but as @Bilal2453 mentioned, I think it would be best for it to be its own client option.
Good enough for 2.x. I can always rewrite it in 3.0. I would rather have this as
false
by default though.
I just realized this PR is for 3.0. Awkward.
I honestly don't like the code. In particular: the concatenation before calling API:log
then the potential interpolation of an empty string into %s
in API:log
.
Understandable. Would adding something like this be better?
function API:logWithMessage(level, res, method, url, message)
return self._client:log(level, '%i - %s : %s %s\n%s', res.code, res.reason, method, url, message)
end
My worry is that this duplicates code.
...or perhaps something like this?
function API:log(level, res, method, url, extra)
local base = self._client:log(level, '%i - %s : %s %s', res.code, res.reason, method, url)
if extra ~= nil then
return base .. '\n' .. extra
else
return base
end
end