Skip to content

Commit e72e396

Browse files
authored
fix: should return default value when env is empty string (#758)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Refactor - Enhanced environment variable handling to trim extra whitespace and improve default value checks for more robust configuration processing. - Tests - Expanded test coverage to validate default behavior, type conversions, and error handling for various environment variable scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 3869742 commit e72e396

File tree

2 files changed

+97
-2
lines changed

2 files changed

+97
-2
lines changed

app/common/EnvUtil.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ export function env(key: string, valueType: ValueType, defaultValue: string): st
44
export function env(key: string, valueType: ValueType, defaultValue: boolean): boolean;
55
export function env(key: string, valueType: ValueType, defaultValue: number): number;
66
export function env(key: string, valueType: ValueType, defaultValue: string | boolean | number): string | boolean | number {
7-
const value = process.env[key];
8-
if (value === undefined) {
7+
let value = process.env[key];
8+
if (typeof value === 'string') {
9+
value = value.trim();
10+
}
11+
if (!value) {
912
return defaultValue;
1013
}
1114

test/common/EnvUtil.test.ts

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { strict as assert } from 'node:assert';
2+
import { mock } from 'egg-mock/bootstrap';
3+
import { env } from '../../app/common/EnvUtil';
4+
5+
describe('test/common/EnvUtil.test.ts', () => {
6+
beforeEach(mock.restore);
7+
8+
describe('env()', () => {
9+
it('should return default value if env is not set', () => {
10+
assert.equal(env('TEST_ENV_STRING', 'string', 'default'), 'default');
11+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), false);
12+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 0);
13+
});
14+
15+
it('should return env value if env is set to empty string', () => {
16+
mock(process.env, 'TEST_ENV_STRING', '');
17+
mock(process.env, 'TEST_ENV_BOOLEAN', '');
18+
mock(process.env, 'TEST_ENV_NUMBER', '');
19+
20+
assert.equal(env('TEST_ENV_STRING', 'string', ''), '');
21+
assert.equal(env('TEST_ENV_STRING', 'string', 'default string'), 'default string');
22+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', true), true);
23+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), false);
24+
assert.equal(env('TEST_ENV_NUMBER', 'number', 3306), 3306);
25+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 0);
26+
assert.equal(env('TEST_ENV_NUMBER', 'number', -123), -123);
27+
28+
mock(process.env, 'TEST_ENV_STRING', ' ');
29+
mock(process.env, 'TEST_ENV_BOOLEAN', ' \t ');
30+
mock(process.env, 'TEST_ENV_NUMBER', ' \t\t\t ');
31+
32+
assert.equal(env('TEST_ENV_STRING', 'string', ''), '');
33+
assert.equal(env('TEST_ENV_STRING', 'string', 'default string'), 'default string');
34+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', true), true);
35+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), false);
36+
assert.equal(env('TEST_ENV_NUMBER', 'number', 3306), 3306);
37+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 0);
38+
});
39+
40+
it('should throw error if env is set to invalid value', () => {
41+
mock(process.env, 'TEST_ENV_BOOLEAN', 'invalid');
42+
assert.throws(() => env('TEST_ENV_BOOLEAN', 'boolean', false), /Invalid boolean value: invalid on process.env.TEST_ENV_BOOLEAN/);
43+
44+
mock(process.env, 'TEST_ENV_NUMBER', 'invalid');
45+
assert.throws(() => env('TEST_ENV_NUMBER', 'number', 0), /Invalid number value: invalid on process.env.TEST_ENV_NUMBER/);
46+
47+
mock(process.env, 'TEST_ENV_NUMBER', 'abc');
48+
assert.throws(() => env('TEST_ENV_NUMBER', 'number', 0), /Invalid number value: abc on process.env.TEST_ENV_NUMBER/);
49+
});
50+
51+
it('should throw error if value type is invalid', () => {
52+
mock(process.env, 'TEST_ENV_STRING', '123');
53+
assert.throws(() => (env as any)('TEST_ENV_STRING', 'float', 'default'), /Invalid value type: float/);
54+
});
55+
56+
it('should work with string value', () => {
57+
mock(process.env, 'TEST_ENV_STRING', 'http://localhost:3000');
58+
assert.equal(env('TEST_ENV_STRING', 'string', 'default'), 'http://localhost:3000');
59+
60+
mock(process.env, 'TEST_ENV_STRING', ' ');
61+
assert.equal(env('TEST_ENV_STRING', 'string', 'default'), 'default');
62+
});
63+
64+
it('should work with boolean value', () => {
65+
mock(process.env, 'TEST_ENV_BOOLEAN', 'true');
66+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), true);
67+
68+
mock(process.env, 'TEST_ENV_BOOLEAN', 'false');
69+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', true), false);
70+
71+
mock(process.env, 'TEST_ENV_BOOLEAN', '1');
72+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', false), true);
73+
74+
mock(process.env, 'TEST_ENV_BOOLEAN', '0');
75+
assert.equal(env('TEST_ENV_BOOLEAN', 'boolean', true), false);
76+
});
77+
78+
it('should work with number value', () => {
79+
mock(process.env, 'TEST_ENV_NUMBER', '123');
80+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 123);
81+
82+
mock(process.env, 'TEST_ENV_NUMBER', '-123');
83+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), -123);
84+
85+
mock(process.env, 'TEST_ENV_NUMBER', '123.456');
86+
assert.equal(env('TEST_ENV_NUMBER', 'number', 0), 123.456);
87+
88+
mock(process.env, 'TEST_ENV_NUMBER', '0');
89+
assert.equal(env('TEST_ENV_NUMBER', 'number', 10), 0);
90+
});
91+
});
92+
});

0 commit comments

Comments
 (0)