Skip to content

fix(HTTPClient): always emit RETRY_AFTER for 429s; recognise OpenAI headers #228

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.

Sign up for GitHub

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

Open
plenarius wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(HTTPClient): always emit RETRY_AFTER for 429s; recognise OpenAI headers #228

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 101 additions & 26 deletions Plugins/HTTPClient/HTTPClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,87 @@ namespace Core {
using namespace NWNXLib;
using namespace NWNXLib::API;

namespace {

// Parse a duration string like "1.5s", "200ms", "1m30s" into milliseconds.
// Used for OpenAI-style x-ratelimit-reset-* headers. Returns -1 if unparseable.
int64_t ParseDurationToMs(const std::string& s)
{
int64_t totalMs = 0;
size_t i = 0;
bool any = false;
while (i < s.size())
{
size_t numStart = i;
while (i < s.size() && (std::isdigit(static_cast<unsigned char>(s[i])) || s[i] == '.'))
++i;
if (numStart == i) return -1;
double value;
try { value = std::stod(s.substr(numStart, i - numStart)); }
catch (...) { return -1; }
std::string unit;
while (i < s.size() && std::isalpha(static_cast<unsigned char>(s[i])))
unit += static_cast<char>(std::tolower(static_cast<unsigned char>(s[i++])));
if (unit == "ms") totalMs += static_cast<int64_t>(value);
else if (unit == "s" || unit.empty()) totalMs += static_cast<int64_t>(value * 1000.0);
else if (unit == "m") totalMs += static_cast<int64_t>(value * 60.0 * 1000.0);
else if (unit == "h") totalMs += static_cast<int64_t>(value * 3600.0 * 1000.0);
else return -1;
any = true;
}
return any ? totalMs : -1;
}

// Try every retry-hint header we recognize, in priority order, and return ms.
// Returns -1 if no usable signal is present.
//
// Why every provider needs its own probe:
// - OpenAI 429s (especially insufficient_quota) often omit Retry-After
// entirely but include `retry-after-ms` and per-bucket `x-ratelimit-reset-*`.
// - Discord uses float-seconds in `X-RateLimit-Reset-After` (more precise
// than the integer-seconds `Retry-After`).
// - RFC 7231 Retry-After can be either delay-seconds OR an HTTP-date; we
// handle the numeric case and ignore dates rather than crash.
int64_t ComputeRetryAfterMs(const httplib::Response& response)
{
if (response.has_header("Retry-After-Ms"))
{
try { return std::stoll(response.get_header_value("Retry-After-Ms")); }
catch (...) {}
}
if (response.has_header("X-RateLimit-Reset-After"))
{
try { return static_cast<int64_t>(std::stof(response.get_header_value("X-RateLimit-Reset-After")) * 1000.0f); }
catch (...) {}
}
if (response.has_header("Retry-After"))
{
try { return static_cast<int64_t>(std::stof(response.get_header_value("Retry-After")) * 1000.0f); }
catch (...) {} // HTTP-date form — fall through
}
int64_t openAiMs = -1;
for (const char* h : {"X-RateLimit-Reset-Requests", "X-RateLimit-Reset-Tokens"})
{
if (response.has_header(h))
{
int64_t v = ParseDurationToMs(response.get_header_value(h));
if (v >= 0 && (openAiMs < 0 || v < openAiMs))
openAiMs = v;
}
}
return openAiMs;
}

// Bounds applied to whatever ComputeRetryAfterMs returns before we emit it.
// Default: a sane wait when no header is parseable (the original bug — the
// scripts saw 0 and busy-looped). Min: never busy-loop. Max: never park
// requests for absurd durations even if a server returns garbage.
constexpr int64_t kRetryDefaultMs = 5000;
constexpr int64_t kRetryMinMs = 1000;
constexpr int64_t kRetryMaxMs = 60LL * 60 * 1000;

} // namespace

static std::unique_ptr<httplib::Result> GetResult(const Request&);
static httplib::Headers ParseHeaderString(const std::string&);
static int s_clientRequestId = 0;
Expand Down Expand Up @@ -202,9 +283,8 @@ void PerformRequest(const Request &client_req)
response.status == 201 ||
response.status == 204 || response.status == 429)
{
// Discord sends your rate limit information even on success so you can stagger calls if you want
// This header also lets us know it's Discord not Slack, important because Discord sends RETRY_AFTER
// in milliseconds and Slack sends it as seconds.
// Discord rate-limit metadata: emitted on every response (incl.
// success) so callers can pace requests preemptively.
if (response.has_header("X-RateLimit-Limit"))
{
MessageBus::Broadcast(
Expand All @@ -222,32 +302,26 @@ void PerformRequest(const Request &client_req)
{"RATELIMIT_RESET",
response.get_header_value(
"X-RateLimit-Reset")});
if (response.has_header("Retry-After"))
MessageBus::Broadcast(
"NWNX_EVENT_PUSH_EVENT_DATA",
{"RETRY_AFTER",
response.get_header_value(
"Retry-After")});
else if (response.has_header("Retry-At"))
{
MessageBus::Broadcast(
"NWNX_EVENT_PUSH_EVENT_DATA",
{"RETRY_AFTER",
response.get_header_value(
"Retry-At")});
}
}
// Slack rate limited
else if (response.has_header("Retry-After"))

// Compute retry delay (ms) from any provider's hint headers.
// For non-429 responses we only emit RETRY_AFTER if the server
// actually sent a hint (Discord does this on successes too).
// For 429 we ALWAYS emit a value, falling back to a sane
// default — otherwise downstream scripts read an undefined
// event tag, get 0, and busy-loop the main thread.
int64_t retryMs = ComputeRetryAfterMs(response);
if (response.status == 429 && retryMs <= 0)
retryMs = kRetryDefaultMs;
if (retryMs > 0)
{
float fSlackRetry =
stof(response.get_header_value(
"Retry-After")) * 1000.0f;
if (retryMs < kRetryMinMs) retryMs = kRetryMinMs;
if (retryMs > kRetryMaxMs) retryMs = kRetryMaxMs;
MessageBus::Broadcast(
"NWNX_EVENT_PUSH_EVENT_DATA",
{"RETRY_AFTER",
std::to_string(fSlackRetry)});
{"RETRY_AFTER", std::to_string(retryMs)});
}

if (response.status != 429)
{
MessageBus::Broadcast(
Expand All @@ -265,8 +339,9 @@ void PerformRequest(const Request &client_req)
{"NWNX_ON_HTTPCLIENT_FAILED",
moduleOid});
LOG_WARNING(
"HTTP Client Request to '%s%s' failed, rate limited.",
client_req.host, client_req.path);
"HTTP Client Request to '%s%s' failed, rate limited (retry in %lldms).",
client_req.host, client_req.path,
static_cast<long long>(retryMs));
}
}
else
Expand Down
Loading