Skip to content
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.

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

tweak(server/http): provide error response headers #2804

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

Conversation

D4isDAVID
Copy link
Contributor

Goal of this PR

In the source code, an HTTP request failure is both failure to connect, but also response codes such as 4xx or 5xx. In both cases, response headers are not provided to scripts.

The problem is that when HTTP servers respond with an error code such as 4xx or 5xx, they may also provide response headers which may contain valuable information - such as the Discord API providing rate limit information when getting rate limited.

The goal is to provide those response headers when requests fail.

How is this PR achieving the goal

Always creates the response data, not only when the request is successful, and passes it to the callback event also when the request has failed.

Seeing as this may be constituted as a breaking change, I passed it as a new callback parameter instead of the existing headers parameter.

This PR applies to the following area(s)

Server, ScRT: Lua

Successfully tested on

Platforms: Windows

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

@github-actions github-actions bot added ScRT: Lua Issues/PRs related to the Lua scripting runtime invalid Requires changes before it's considered valid and can be (re)triaged labels Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Requires changes before it's considered valid and can be (re)triaged ScRT: Lua Issues/PRs related to the Lua scripting runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant