Skip to content
Draft
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
138 changes: 138 additions & 0 deletions src/lib/http-server/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,3 +589,141 @@ test('maxBodySize: rejects oversized requests', async function() {

await server.stop();
}, 30000);

test('KeetaNetCombinedAnchorHTTPServer: combines routes from multiple servers', async function() {
const serverA = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /api/server-a': async function() {
return({ output: JSON.stringify({ server: 'A' }), statusCode: 200 });
}
});
}
})({ port: 0 });

const serverB = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /api/server-b': async function() {
return({ output: JSON.stringify({ server: 'B' }), statusCode: 200 });
}
});
}
})({ port: 0 });

await using combined = new HTTPServer.KeetaNetCombinedAnchorHTTPServer({
port: 0,
servers: [serverA, serverB]
});

await combined.start();

/*
* Routes from both child servers should be accessible via the combined server.
*/
const responseA = await testHTTPRequest(combined.url, '/api/server-a', 'GET');
expect(responseA.code).toBe(200);
expect(responseA.body).toMatchObject({ server: 'A' });

const responseB = await testHTTPRequest(combined.url, '/api/server-b', 'GET');
expect(responseB.code).toBe(200);
expect(responseB.body).toMatchObject({ server: 'B' });

/*
* A route that doesn't exist on either child should return 404.
*/
const responseMissing = await testHTTPRequest(combined.url, '/api/missing', 'GET');
expect(responseMissing.code).toBe(404);

/*
* The combined server's URL should be propagated to child instances after start.
*/
expect(serverA.url).toBe(combined.url);
expect(serverB.url).toBe(combined.url);
}, 30000);

test('KeetaNetCombinedAnchorHTTPServer: later child routes overwrite earlier ones on conflict', async function() {
const serverA = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /api/conflict': async function() {
return({ output: JSON.stringify({ from: 'A' }), statusCode: 200 });
}
});
}
})({ port: 0 });

const serverB = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /api/conflict': async function() {
return({ output: JSON.stringify({ from: 'B' }), statusCode: 200 });
}
});
}
})({ port: 0 });

await using combined = new HTTPServer.KeetaNetCombinedAnchorHTTPServer({
port: 0,
servers: [serverA, serverB]
});

await combined.start();

const response = await testHTTPRequest(combined.url, '/api/conflict', 'GET');
expect(response.code).toBe(200);
expect(response.body).toMatchObject({ from: 'B' });
}, 30000);

test('KeetaNetCombinedAnchorHTTPServer: url set on combined server propagates to children', async function() {
const child = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /health': async function() {
return({ output: JSON.stringify({ ok: true }), statusCode: 200 });
}
});
}
})({ port: 0 });

const customURL = 'https://anchor.example.com/';

await using combined = new HTTPServer.KeetaNetCombinedAnchorHTTPServer({
port: 0,
url: customURL,
servers: [child]
});

await combined.start();

expect(combined.url).toBe(customURL);

/*
* The custom URL should also be propagated to the child.
*/
expect(child.url).toBe(customURL);
}, 30000);

test('KeetaNetCombinedAnchorHTTPServer: throws when a child has a conflicting URL', async function() {
const conflictingURL = 'https://other.example.com/';

const child = new (class extends HTTPServer.KeetaNetAnchorHTTPServer<HTTPServer.KeetaAnchorHTTPServerConfig> {
protected async initRoutes(): Promise<HTTPServer.Routes> {
return({
'GET /health': async function() {
return({ output: JSON.stringify({ ok: true }), statusCode: 200 });
}
});
}
})({ port: 0, url: conflictingURL });

const customURL = 'https://anchor.example.com/';

await using combined = new HTTPServer.KeetaNetCombinedAnchorHTTPServer({
port: 0,
url: customURL,
servers: [child]
});

await expect(combined.start()).rejects.toThrow(`Child server url "${conflictingURL}" does not match combined server url "${customURL}"`);
}, 30000);
105 changes: 87 additions & 18 deletions src/lib/http-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export abstract class KeetaNetAnchorHTTPServer<ConfigType extends KeetaAnchorHTT
#serverPromise?: Promise<void>;
#server?: http.Server;
#urlParts: undefined | { hostname?: string; port?: number; protocol?: string; };
#url: undefined | string | URL | ((object: this) => string);
#url: undefined | string | URL | ((object: KeetaNetAnchorHTTPServer) => string);
readonly #config: ConfigType;

constructor(config: ConfigType) {
Expand All @@ -83,18 +83,6 @@ export abstract class KeetaNetAnchorHTTPServer<ConfigType extends KeetaAnchorHTT
this.#url = config.url;
this.#urlParts = undefined;
} else if (typeof config.url === 'function') {
/**
* The parameter for the call back is typed as
* `this`, which means any subclass is typed
* but the interface can't identify that --
* instead it types it as the base class
* (KeetaNetAnchorHTTPServer), which means it
* can't be assigned to the type of `#url`
* without overriding the type check. However,
* we know that `this` will be at least
* compatible with the base class.
*/
// @ts-ignore
this.#url = config.url;
this.#urlParts = undefined;
} else {
Expand Down Expand Up @@ -651,17 +639,16 @@ export abstract class KeetaNetAnchorHTTPServer<ConfigType extends KeetaAnchorHTT
* setting a custom URL.
*/
get url(): string {
if (this.port === 0 || this.#server === undefined) {
throw(new Error('Server not started'));
}

if (this.#url !== undefined) {
let newURL: string;
if (typeof this.#url === 'string') {
newURL = this.#url;
} else if (this.#url instanceof URL || ('port' in this.#url && 'hostname' in this.#url && 'toString' in this.#url)) {
newURL = this.#url.toString();
} else if (typeof this.#url === 'function') {
if (this.port === 0 || this.#server === undefined) {
throw(new Error('Server not started'));
}
newURL = this.#url(this);
} else {
assertNever(this.#url);
Expand All @@ -676,6 +663,10 @@ export abstract class KeetaNetAnchorHTTPServer<ConfigType extends KeetaAnchorHTT
return(retval);
}

if (this.port === 0 || this.#server === undefined) {
throw(new Error('Server not started'));
}

const urlObj = new URL('http://localhost');
urlObj.port = String(this.#urlParts?.port ?? this.port);
urlObj.hostname = this.#urlParts?.hostname ?? 'localhost';
Expand All @@ -688,12 +679,90 @@ export abstract class KeetaNetAnchorHTTPServer<ConfigType extends KeetaAnchorHTT
return(retval);
}

set url(value: undefined | string | URL | ((object: this) => string)) {
set url(value: undefined | string | URL | ((object: KeetaNetAnchorHTTPServer) => string)) {
this.#urlParts = undefined;
this.#url = value;
}

/**
* Expose the config so that subclasses can access it without
* needing to maintain their own copy of the same data.
*/
protected get config(): ConfigType {
return(this.#config);
}

/**
* Build the routes for this server by calling initRoutes with the
* config that was provided at construction time. This is used
* internally by KeetaNetCombinedAnchorHTTPServer to gather routes
* from each child server.
*/
protected buildRoutes(): Promise<Routes> {
return(this.initRoutes(this.#config));
}
Comment on lines +701 to +703
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have a method that overrides initRoutes? It does the same thing

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildRoutes() is a protected proxy that packages initRoutes(this.#config) into a no-argument call. It's needed because initRoutes is generic over ConfigType — you can't call it on an arbitrary child instance from outside the class without knowing (or storing) its config type. The method allows KeetaNetCombinedAnchorHTTPServer to invoke the correct initRoutes on each child via bracket notation without a cast that loses type information.


[Symbol.asyncDispose](): Promise<void> {
return(this.stop());
}
}

export interface KeetaAnchorCombinedHTTPServerConfig extends KeetaAnchorHTTPServerConfig {
/**
* The list of servers whose routes should be combined.
*/
servers: KeetaNetAnchorHTTPServer[];
}

/**
* A KeetaNetAnchorHTTPServer that combines the routes of multiple
* KeetaNetAnchorHTTPServer instances into a single HTTP server.
*
* When the combined server starts it propagates its URL to all child
* server instances so that callers holding references to those children
* can retrieve the correct URL via child.url.
*/
export class KeetaNetCombinedAnchorHTTPServer extends KeetaNetAnchorHTTPServer<KeetaAnchorCombinedHTTPServerConfig> {
protected async initRoutes(config: KeetaAnchorCombinedHTTPServerConfig): Promise<Routes> {
const combined: Routes = {};

for (const child of config.servers) {
// buildRoutes() is protected; bracket notation is used here to access
// it on an arbitrary KeetaNetAnchorHTTPServer instance.
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
const childRoutes: Routes = await (child as unknown as { buildRoutes(): Promise<Routes> }).buildRoutes();
Object.assign(combined, childRoutes);
}

return(combined);
}

override async start(): Promise<void> {
await super.start();

const url = this.url;

/*
* Propagate the combined server's URL to all child server instances.
* If a child already has a URL set it must match the combined server's
* URL; a mismatch indicates a misconfiguration.
*/
for (const child of this.config.servers) {
let childURL: string | undefined;
try {
childURL = child.url;
} catch (err) {
if (!(err instanceof Error && err.message === 'Server not started')) {
throw(err);
}
/* child has no static URL set; the URL will be propagated below */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would this throw? If it does, should we check the specific error and throw unknown?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The url getter throws new Error('Server not started') when no static URL is set and the server hasn't started. Fixed in 3973da7: the catch now explicitly re-throws anything that isn't that specific error, so unexpected errors are not silently swallowed.

}

if (childURL !== undefined && childURL !== url) {
throw(new Error(`Child server url "${childURL}" does not match combined server url "${url}"`));
}

child.url = url;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the child already has a URL set? We should ensure they either do not or it is the same.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added validation in ac464cb. Before propagating the URL, start() now tries to read each child's existing URL. If the child already has a static URL set and it differs from the combined server's URL, it throws: Error: Child server url "..." does not match combined server url "...". A test for this case is also added.

}
}
}