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

feat: try other logger ports #3534

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e245cf5
feat/try other logger ports
DevanceJ Mar 14, 2024
9b8e9e9
made some changes as per the code review and added unit tests for Logger
DevanceJ Mar 15, 2024
6d5fd76
transfered server functions to utils and added tests
DevanceJ Mar 15, 2024
7c630c7
removed old logger tests
DevanceJ Mar 15, 2024
1254b70
changed unit tests and got rid of stub
DevanceJ Mar 18, 2024
feee8fe
creating a server to make the tests more robust
DevanceJ Mar 19, 2024
32809e8
updated logger class according new param passed to start
DevanceJ Mar 19, 2024
a8f6a88
Merge branch 'main' into feat/try-other-ports
BlackHole1 Mar 20, 2024
59d6033
Merge branch 'main' into feat/try-other-ports
BlackHole1 Mar 21, 2024
7df57bf
bumped dependency version of core/utils to 7.3.1
DevanceJ Mar 21, 2024
39e4961
Merge branch 'main' into feat/try-other-ports
DevanceJ Mar 23, 2024
dc4fe2f
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 2, 2024
012bfe2
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 2, 2024
f5f9056
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 6, 2024
c6bc940
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 13, 2024
c38e6ce
bumped the version to resolve tests
DevanceJ Apr 15, 2024
4aff4c7
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 20, 2024
caae0a2
refactor portoccupied function and tests
DevanceJ Apr 20, 2024
57275fe
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 22, 2024
7a44cbb
refactors webpack plugin to avoid any ts errors
DevanceJ Apr 23, 2024
547acf9
Merge branch 'main' into feat/try-other-ports
DevanceJ Apr 28, 2024
bac9781
Merge branch 'main' into feat/try-other-ports
DevanceJ May 2, 2024
86850d9
Merge branch 'main' into feat/try-other-ports
DevanceJ May 4, 2024
2f15245
Merge branch 'main' into feat/try-other-ports
erickzhao May 8, 2024
825a426
fixes ts errors?
DevanceJ May 29, 2024
dc8dacd
Merge branch 'main' into feat/try-other-ports
DevanceJ Jun 7, 2024
d372fb2
Merge branch 'main' into feat/try-other-ports
DevanceJ Jun 11, 2024
ead16ac
Merge branch 'main' into feat/try-other-ports
DevanceJ Jun 18, 2024
5b411b0
Merge branch 'main' into feat/try-other-ports
DevanceJ Jun 23, 2024
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
4 changes: 2 additions & 2 deletions packages/plugin/webpack/src/WebpackPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,9 @@ the generated files). Instead, it is ${JSON.stringify(pj.main)}`);

await fs.remove(this.baseDir);

const logger = new Logger(this.loggerPort);
const logger = new Logger();
this.loggers.push(logger);
await logger.start();
this.loggerPort = await logger.start(this.loggerPort);

return {
tasks: [
Expand Down
1 change: 1 addition & 0 deletions packages/utils/core-utils/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './rebuild';
export * from './electron-version';
export * from './yarn-or-npm';
export * from './port';
41 changes: 41 additions & 0 deletions packages/utils/core-utils/src/port.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import http from 'http';

/**
* Check if a port is occupied.
* @returns boolean promise that resolves to true if the port is available, false otherwise.
DevanceJ marked this conversation as resolved.
Show resolved Hide resolved
*/
export const portOccupied = async (port: number): Promise<void> => {
return new Promise<void>((resolve, reject) => {
const server = http.createServer().listen(port);
server.on('listening', () => {
server.close();
resolve();
DevanceJ marked this conversation as resolved.
Show resolved Hide resolved
});

server.on('error', (error) => {
if ('code' in error && (error as NodeJS.ErrnoException).code === 'EADDRINUSE') {
reject(new Error(`port: ${port} is occupied`));
} else {
reject(error);
}
});
});
};

/**
* Find an available port for web UI.
* @returns the port number.
*/
export const findAvailablePort = async (initialPort: number): Promise<number> => {
const maxPort = initialPort + 10;

for (let p = initialPort; p <= maxPort; p++) {
try {
await portOccupied(p);
return p;
} catch (_err) {
// Pass
}
}
throw new Error(`Could not find an available port between ${initialPort} and ${maxPort}. Please free up a port and try again.`);
};
68 changes: 68 additions & 0 deletions packages/utils/core-utils/test/port_spec.ts
BlackHole1 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import net from 'node:net';

import { expect } from 'chai';

import { findAvailablePort, portOccupied } from '../src/port';

const usePort = (port: number) => {
const server = net.createServer();
server.on('error', () => {
// pass
});
server.listen(port);
return () => {
server.close();
};
};

const usePorts = (port: number, endPort: number) => {
const releases: ReturnType<typeof usePort>[] = [];
for (let i = port; i <= endPort; i++) {
releases.push(usePort(i));
}
return () => {
releases.forEach((release) => release());
};
};

describe('Port tests', () => {
describe('portOccupied', () => {
it('should resolve to true if the port is available', async () => {
const port = 49152;
const result = await portOccupied(port);
expect(result).to.not.throw;
});
it('should reject if the port is occupied', async () => {
const port = 48143;
const releasePort = usePort(port);
try {
await portOccupied(port);
} catch (error) {
expect((error as Error).message).to.equal(`port: ${port} is occupied`);
} finally {
releasePort();
}
});
});

describe('findAvailablePort', () => {
it('should find an available port', async () => {
const initialPort = 51155;
const port = await findAvailablePort(initialPort);
expect(port).gte(initialPort);
});
it('should throw an error if no available port is found', async () => {
const initialPort = 53024;
const releasePort = usePorts(initialPort, initialPort + 10);
try {
await findAvailablePort(initialPort);
} catch (error) {
expect((error as Error).message).to.equal(
`Could not find an available port between ${initialPort} and ${initialPort + 10}. Please free up a port and try again.`
);
} finally {
releasePort();
}
});
});
});
1 change: 1 addition & 0 deletions packages/utils/web-multi-logger/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"main": "dist/Logger.js",
"typings": "dist/Logger.d.ts",
"dependencies": {
"@electron-forge/core-utils": "7.3.0",
"express": "^4.17.1",
"express-ws": "^5.0.2",
"xterm": "^4.9.0",
Expand Down
10 changes: 5 additions & 5 deletions packages/utils/web-multi-logger/src/Logger.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import http from 'http';
import path from 'path';

import { findAvailablePort } from '@electron-forge/core-utils';
import express from 'express';
import ews from 'express-ws';

Expand Down Expand Up @@ -33,7 +34,6 @@ export default class Logger {
// I assume this endpoint is just a no-op needed for some reason.
});
}

/**
* Creates a new tab with the given name, the name should be human readable
* it will be used as the tab title in the front end.
Expand All @@ -49,10 +49,10 @@ export default class Logger {
*
* @returns the port number
*/
start(): Promise<number> {
return new Promise<number>((resolve) => {
this.server = this.app.listen(this.port, () => resolve(this.port));
});
async start(loggerPort: number): Promise<number> {
loggerPort = await findAvailablePort(loggerPort);
this.server = this.app.listen(loggerPort);
return loggerPort;
}

/**
Expand Down