Skip to content

Fixed conversion of environment variables to number types with Claude Code 2 #154

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 Claude Code 2 #154

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
24 changes: 19 additions & 5 deletions src/env.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
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 parseEnvInteger = (envVar: string, defaultValue: number): number => {
const value = process.env[envVar];
if (value === undefined) {
return defaultValue;
}
const parsed = toSafeInteger(value);
if (parsed === undefined) {
throw new Error(`Invalid ${envVar} environment variable: "${value}" is not a valid integer`);
}
return parsed;
};

const headersTimeout = parseEnvInteger('HEADERS_TIMEOUT', 10000);
const requestTimeout = parseEnvInteger('REQUEST_TIMEOUT', 30000);
const keepAliveTimeout = parseEnvInteger('KEEP_ALIVE_TIMEOUT', 5000);

if (headersTimeout <= keepAliveTimeout) {
throw new Error(
Expand All @@ -19,9 +33,9 @@ export const environment = {
headersTimeout,
keepAliveTimeout,
logLevel: process.env['LOG_LEVEL'] || 'info',
maxDelay: Number(process.env['MAX_DELAY']) || 10000,
maxDelay: parseEnvInteger('MAX_DELAY', 10000),
nodeEnv: process.env['NODE_ENV'] || 'development',
origin: process.env['ORIGIN'] || '*',
port: Number(process.env['PORT']) || 8000,
port: parseEnvInteger('PORT', 8000),
requestTimeout,
};
48 changes: 48 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,52 @@ describe('Environment configuration', () => {
requestTimeout: 40000,
});
});

it('should throw an error when HEADERS_TIMEOUT is not a valid integer', async () => {
process.env.HEADERS_TIMEOUT = 'invalid';

await expect(loadEnv()).rejects.toThrow(
'Invalid HEADERS_TIMEOUT environment variable: "invalid" is not a valid integer'
);
});

it('should throw an error when REQUEST_TIMEOUT is not a valid integer', async () => {
process.env.REQUEST_TIMEOUT = '1.5';

await expect(loadEnv()).rejects.toThrow(
'Invalid REQUEST_TIMEOUT environment variable: "1.5" is not a valid integer'
);
});

it('should throw an error when KEEP_ALIVE_TIMEOUT is not a valid integer', async () => {
process.env.KEEP_ALIVE_TIMEOUT = '2e10';

await expect(loadEnv()).rejects.toThrow(
'Invalid KEEP_ALIVE_TIMEOUT environment variable: "2e10" is not a valid integer'
);
});

it('should throw an error when MAX_DELAY is not a valid integer', async () => {
process.env.MAX_DELAY = 'abc';

await expect(loadEnv()).rejects.toThrow(
'Invalid MAX_DELAY environment variable: "abc" is not a valid integer'
);
});

it('should throw an error when PORT is not a valid integer', async () => {
process.env.PORT = '8000.0';

await expect(loadEnv()).rejects.toThrow(
'Invalid PORT environment variable: "8000.0" is not a valid integer'
);
});

it('should throw an error when integer is out of safe range', async () => {
process.env.PORT = '9007199254740992';

await expect(loadEnv()).rejects.toThrow(
'Invalid PORT environment variable: "9007199254740992" is not a valid integer'
);
});
Comment on lines +84 to +130

@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.

medium

These tests are great for ensuring invalid values are handled correctly. To improve maintainability and reduce code duplication, you could parameterize these tests using it.each, which you're already using elsewhere in the test suite. This would make the test suite more concise and easier to extend with new invalid cases in the future.

  describe('when an integer environment variable is invalid', () => {
    const testCases = [
      { name: 'not an integer', envVar: 'HEADERS_TIMEOUT', value: 'invalid' },
      { name: 'a float string', envVar: 'REQUEST_TIMEOUT', value: '1.5' },
      { name: 'scientific notation', envVar: 'KEEP_ALIVE_TIMEOUT', value: '2e10' },
      { name: 'a non-numeric string', envVar: 'MAX_DELAY', value: 'abc' },
      { name: 'a float string with .0', envVar: 'PORT', value: '8000.0' },
      { name: 'an out of safe range integer', envVar: 'PORT', value: '9007199254740992' },
    ];

    it.each(testCases)('should throw an error when $envVar is $name ($value)', async ({ envVar, value }) => {
      process.env[envVar] = value;

      await expect(loadEnv()).rejects.toThrow(
        `Invalid ${envVar} environment variable: "${value}" is not a valid integer`
      );
    });
  });

});