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

Allow configuring RequestHandler to use HTTP instead of HTTPS #1193

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

molenzwiebel
Copy link

My use case was to tunnel all Eris traffic through a central rate limiter (Twilight's http-proxy), which uses HTTP instead of HTTPS. Given that we can already configure the domain and base URL, being able to configure whether to use HTTPS or HTTP seemed logical.

I've named it https and enabled it by default, to ensure concious opt-out by doing https: false. A name like useHttps might be more appropriate.

Could also consider storing the request method (https or http module) as an instance variable on RequestHandler instead of doing the ternary on each request, but I figured this'd be fine.

JSDoc and TypeScript typings should be good.

@abalabahaha abalabahaha changed the base branch from master to dev May 1, 2021 19:33
@tbnritzdoge
Copy link

Should we not just take the protocol from the rest URL provided?

@molenzwiebel
Copy link
Author

The domain doesn't have a protocol prefixed. Using it would probably be neater, but it would break backwards compat.

lib/Client.js Outdated Show resolved Hide resolved
DonovanDMC added a commit to DonovanArchive/ErisPRUpdateBot that referenced this pull request Jul 18, 2022
@bsian03
Copy link
Collaborator

bsian03 commented Jun 18, 2024

Request from @HcgRandon - add option to override port for proxy related stuff. Not urgent

@bsian03
Copy link
Collaborator

bsian03 commented Sep 23, 2024

@HcgRandon I added port functionality into this PR, is this essentially what you're currently using to connect to nirn?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants