[PECO-1042] Add proxy support #193
Conversation
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
| // Obtain http agent each time when we need an OAuth client | ||
| // to ensure that we always use a valid agent instance | ||
| const connectionProvider = await this.context.getConnectionProvider(); | ||
| this.agent = await connectionProvider.getAgent(); |
There was a problem hiding this comment.
Does http.Agent automatically establish a connection pool?
There was a problem hiding this comment.
A sort of. It manages a cache of sockets and can re-use them if possible. I don't know if we can call it a connection pool, but it serves a similar purpose
| protocol: proxyProtocol, | ||
| }); | ||
| } else { | ||
| this.agent = options.https ? new https.Agent(httpsAgentOptions) : new http.Agent(httpAgentOptions); |
There was a problem hiding this comment.
This nested if-then logic confuses me. Under what circumstances is line 71 executed? Is it just when a proxy isn't set as part of options?
If so, for readability I'd recommend swapping the order of the conditions. I'd also avoid the nested if by encapsulating the check on line 71 in its own function that returns either an httpAgentOptions or httpsAgentOptions when passed the options object. So the code would look like this:
if (options.proxy == undefined) {
this.agent = buildAgentFromOptions(options)
} else {
const proxyUrl = buildProxyUrl(options.proxy)
...
}There was a problem hiding this comment.
Makes sense. Will split this code a bit 👍
There was a problem hiding this comment.
I did it a bit differently, but it also should be good: ad01894 Please take one more look
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Depends on #191
Tests(manual for now, proper tests will be added later)