search

krisppurg / dimscord

A Discord Bot & REST Library for Nim.
https://krisppurg.github.io/dimscord/
MIT License
222 stars 22 forks source link

close http client #74

Closed t8m8 closed 2 years ago

t8m8 commented 2 years ago

HTTP client should be closed because it consumes file descriptor

krisppurg commented 2 years ago

Client should close in line 248, I've had issues in the past where there were problems with client.close before parsing json and error handling.

t8m8 commented 2 years ago

I understood. I moved it to line 248.

krisppurg commented 2 years ago

@t8m8, sorry for delay.

Double checking have you tested this?

And could you try doing a ratelimit test on this? I've in the past had issues with ratelimiting where client.close was the issue, so im not 100% sure if it might cause issues.

And also try testing getGuildAuditLogs(...), since audit log data is known to be chunky depending on how much entries there are.

Just make sure ratelimits are working fine as usual and other endpoints.

t8m8 commented 2 years ago

@krisppurg I've been testing it and so far it seems to be working fine.

For my information, when you had issues with ratelimiting before, what kind of issues did you have?

krisppurg commented 2 years ago

No need to worry about those issues, removing client.close() was the solution. I believe this was in May 2020