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

Ensure the supervisor can start even when http_proxy is set #5825

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion extensions/positron-supervisor/src/KallichoreAdapterApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class KCApi implements KallichoreAdapterApi {
*
* @returns A promise that resolves when the Kallichore server is online.
*/
async ensureStarted(): Promise<void> {
public async ensureStarted(): Promise<void> {

// If the server is already started, we're done
if (this._started.isOpen()) {
Expand Down Expand Up @@ -290,6 +290,16 @@ export class KCApi implements KallichoreAdapterApi {
// Wait for the terminal to start and get the PID
await terminal.processId;

// If an HTTP proxy is set, exempt the supervisor from it; since this
// is a local server, we generally don't want to route it through a
// proxy
if (process.env.http_proxy) {
// Add the server's port to the no_proxy list, amending it if it
// already exists
process.env.no_proxy = (process.env.no_proxy ? process.env.no_proxy + ',' : '') + `localhost:${port}`;
this._log.appendLine(`HTTP proxy set to ${process.env.http_proxy}; setting no_proxy to ${process.env.no_proxy} to exempt supervisor`);
}

// Establish the API
this._api.basePath = `http://localhost:${port}`;
this._api.setDefaultAuthentication(bearer);
Expand Down
2 changes: 1 addition & 1 deletion extensions/positron-supervisor/src/test/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Launch tests by running this from the repository root:

```sh
yarn test-extension -l positron-supervisor
npm run test-extension -l positron-supervisor
```
30 changes: 29 additions & 1 deletion extensions/positron-supervisor/src/test/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,35 @@ import { API_INSTANCE } from '../extension';
// Basic test to ensure the server can start and return status, and is the
// correct version as specified in the package.json.
suite('Server', () => {
test('Server starts and connects', async () => {
// Ensure that the server connects even when the http_proxy environment
// variable is set.
//
// NOTE: This test must run first because we only examine this value when
// the server is started; subsequent tests will use the warm server. We
// could improve this by adding some scaffolding to use a fresh server
// instance for every test.
test('Server connects when http_proxy is set', async () => {
// Set the http_proxy environment variable to a non-existent proxy
const oldHttpProxy = process.env.http_proxy;
process.env.http_proxy = 'http://example.com:234';

// Start the server and connect to it
try {
await API_INSTANCE.ensureStarted();
const status = await API_INSTANCE.serverStatus();

// Sanity check a response field
assert.strictEqual(status.sessions, 0);
} finally {
// Reset the http_proxy environment variable
process.env.http_proxy = oldHttpProxy;
}
});

test('Server version is correct', async () => {

// Start the server and connect to it
await API_INSTANCE.ensureStarted();
const status = await API_INSTANCE.serverStatus();

// Read the package.json file to get the version
Expand All @@ -20,4 +47,5 @@ suite('Server', () => {
// package.json version
assert.strictEqual(status.version, pkg.positron.binaryDependencies.kallichore);
});

});
Loading