Skip to content

Fixed conversion of environment variables to number types with GitHub Copilot 2 #155

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.

Sign up for GitHub

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

Open
ryo8000 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed conversion of environment variables to number types with GitHub Copilot 2 #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 71 additions & 5 deletions src/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,72 @@
const headersTimeout = Number(process.env['HEADERS_TIMEOUT']) || 10000;
const requestTimeout = Number(process.env['REQUEST_TIMEOUT']) || 30000;
const keepAliveTimeout = Number(process.env['KEEP_ALIVE_TIMEOUT']) || 5000;
import { toSafeInteger } from './utils/number.js';

const headersTimeoutValue = process.env['HEADERS_TIMEOUT'];
const requestTimeoutValue = process.env['REQUEST_TIMEOUT'];
const keepAliveTimeoutValue = process.env['KEEP_ALIVE_TIMEOUT'];
const maxDelayValue = process.env['MAX_DELAY'];
const portValue = process.env['PORT'];

// Convert environment variables with error handling
const headersTimeout =
headersTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(headersTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid HEADERS_TIMEOUT value: "${headersTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 10000;

const requestTimeout =
requestTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(requestTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid REQUEST_TIMEOUT value: "${requestTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 30000;

const keepAliveTimeout =
keepAliveTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(keepAliveTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid KEEP_ALIVE_TIMEOUT value: "${keepAliveTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 5000;

const maxDelay =
maxDelayValue !== undefined
? (() => {
const parsed = toSafeInteger(maxDelayValue);
if (parsed === undefined) {
throw new Error(`Invalid MAX_DELAY value: "${maxDelayValue}". Must be a valid integer.`);
}
return parsed;
})()
: 10000;

const port =
portValue !== undefined
? (() => {
const parsed = toSafeInteger(portValue);
if (parsed === undefined) {
throw new Error(`Invalid PORT value: "${portValue}". Must be a valid integer.`);
}
return parsed;
})()
: 8000;
Comment on lines +3 to +69

@gemini-code-assist gemini-code-assist Bot Aug 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The logic for parsing and validating numeric environment variables is repeated five times. This code duplication makes the code harder to read and maintain. Any future changes to this logic would need to be applied in all five places, which is error-prone.

To improve this, you can extract the parsing logic into a reusable helper function. This will make the code more concise, readable, and easier to maintain, following the DRY (Don't Repeat Yourself) principle.

Suggested change
const headersTimeoutValue = process.env['HEADERS_TIMEOUT'];
const requestTimeoutValue = process.env['REQUEST_TIMEOUT'];
const keepAliveTimeoutValue = process.env['KEEP_ALIVE_TIMEOUT'];
const maxDelayValue = process.env['MAX_DELAY'];
const portValue = process.env['PORT'];
// Convert environment variables with error handling
const headersTimeout =
headersTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(headersTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid HEADERS_TIMEOUT value: "${headersTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 10000;
const requestTimeout =
requestTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(requestTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid REQUEST_TIMEOUT value: "${requestTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 30000;
const keepAliveTimeout =
keepAliveTimeoutValue !== undefined
? (() => {
const parsed = toSafeInteger(keepAliveTimeoutValue);
if (parsed === undefined) {
throw new Error(
`Invalid KEEP_ALIVE_TIMEOUT value: "${keepAliveTimeoutValue}". Must be a valid integer.`
);
}
return parsed;
})()
: 5000;
const maxDelay =
maxDelayValue !== undefined
? (() => {
const parsed = toSafeInteger(maxDelayValue);
if (parsed === undefined) {
throw new Error(`Invalid MAX_DELAY value: "${maxDelayValue}". Must be a valid integer.`);
}
return parsed;
})()
: 10000;
const port =
portValue !== undefined
? (() => {
const parsed = toSafeInteger(portValue);
if (parsed === undefined) {
throw new Error(`Invalid PORT value: "${portValue}". Must be a valid integer.`);
}
return parsed;
})()
: 8000;
function getNumericEnv(name: string, defaultValue: number): number {
const value = process.env[name];
if (value === undefined) {
return defaultValue;
}
const parsed = toSafeInteger(value);
if (parsed === undefined) {
throw new Error(`Invalid ${name} value: "${value}". Must be a valid integer.`);
}
return parsed;
}
// Convert environment variables with error handling
const headersTimeout = getNumericEnv('HEADERS_TIMEOUT', 10000);
const requestTimeout = getNumericEnv('REQUEST_TIMEOUT', 30000);
const keepAliveTimeout = getNumericEnv('KEEP_ALIVE_TIMEOUT', 5000);
const maxDelay = getNumericEnv('MAX_DELAY', 10000);
const port = getNumericEnv('PORT', 8000);


if (headersTimeout <= keepAliveTimeout) {
throw new Error(
Expand All @@ -19,9 +85,9 @@ export const environment = {
headersTimeout,
keepAliveTimeout,
logLevel: process.env['LOG_LEVEL'] || 'info',
maxDelay: Number(process.env['MAX_DELAY']) || 10000,
maxDelay,
nodeEnv: process.env['NODE_ENV'] || 'development',
origin: process.env['ORIGIN'] || '*',
port: Number(process.env['PORT']) || 8000,
port,
requestTimeout,
};
50 changes: 50 additions & 0 deletions tests/ut/env.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,54 @@ describe('Environment configuration', () => {
requestTimeout: 40000,
});
});

describe('Invalid environment variable values', () => {
it('should throw an error for invalid HEADERS_TIMEOUT value', async () => {
process.env.HEADERS_TIMEOUT = 'invalid';

await expect(loadEnv()).rejects.toThrow(
'Invalid HEADERS_TIMEOUT value: "invalid". Must be a valid integer.'
);
});

it('should throw an error for invalid REQUEST_TIMEOUT value', async () => {
process.env.REQUEST_TIMEOUT = '12.5';

await expect(loadEnv()).rejects.toThrow(
'Invalid REQUEST_TIMEOUT value: "12.5". Must be a valid integer.'
);
});

it('should throw an error for invalid KEEP_ALIVE_TIMEOUT value', async () => {
process.env.KEEP_ALIVE_TIMEOUT = 'abc123';

await expect(loadEnv()).rejects.toThrow(
'Invalid KEEP_ALIVE_TIMEOUT value: "abc123". Must be a valid integer.'
);
});

it('should throw an error for invalid MAX_DELAY value', async () => {
process.env.MAX_DELAY = '1e5';

await expect(loadEnv()).rejects.toThrow(
'Invalid MAX_DELAY value: "1e5". Must be a valid integer.'
);
});

it('should throw an error for invalid PORT value', async () => {
process.env.PORT = 'port8000';

await expect(loadEnv()).rejects.toThrow(
'Invalid PORT value: "port8000". Must be a valid integer.'
);
});

it('should throw an error for out-of-safe-range values', async () => {
process.env.PORT = '9007199254740992'; // Number.MAX_SAFE_INTEGER + 1

await expect(loadEnv()).rejects.toThrow(
'Invalid PORT value: "9007199254740992". Must be a valid integer.'
);
});
});
});