Skip to content

Commit ce98b05

Browse files
MarceloRGoncMarceloRGonc
authored
Improve SMTP connection error handling and make allowed ports configurable (#1885)
Fixes OPS-3307. ## Additional Notes - Refactors SMTP connection error handling - Improves configurability around allowed SMTP ports.
1 parent 687697d commit ce98b05

File tree

7 files changed

+232
-32
lines changed

7 files changed

+232
-32
lines changed

‎packages/blocks/smtp/src/index.ts‎

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { BlockAuth, Property, createBlock } from '@openops/blocks-framework';
22
import { BlockCategory } from '@openops/shared';
33
import { sendEmail } from './lib/actions/send-email';
44
import { smtpCommon } from './lib/common';
5-
6-
const SMTPPorts = [25, 465, 587, 2525];
5+
import { connectionErrorHandler } from './lib/connection-error-handler';
6+
import { getSmtpPortsOptions } from './lib/get-smtp-ports';
77

88
export const smtpAuth = BlockAuth.CustomAuth({
99
authProviderKey: 'SMTP',
@@ -26,15 +26,7 @@ export const smtpAuth = BlockAuth.CustomAuth({
2626
port: Property.StaticDropdown({
2727
displayName: 'Port',
2828
required: true,
29-
options: {
30-
disabled: false,
31-
options: SMTPPorts.map((port) => {
32-
return {
33-
label: port.toString(),
34-
value: port,
35-
};
36-
}),
37-
},
29+
options: getSmtpPortsOptions(),
3830
}),
3931
TLS: Property.Checkbox({
4032
displayName: 'Require TLS?',
@@ -48,33 +40,15 @@ export const smtpAuth = BlockAuth.CustomAuth({
4840
return new Promise((resolve, reject) => {
4941
transporter.verify(function (error, success) {
5042
if (error) {
51-
resolve({ valid: false, error: JSON.stringify(error) });
43+
const errorResult = connectionErrorHandler(error);
44+
resolve(errorResult);
5245
} else {
5346
resolve({ valid: true });
5447
}
5548
});
5649
});
5750
} catch (e) {
58-
const castedError = e as Record<string, unknown>;
59-
const code = castedError?.['code'];
60-
switch (code) {
61-
case 'EDNS':
62-
return {
63-
valid: false,
64-
error: 'SMTP server not found or unreachable with error code: EDNS',
65-
};
66-
case 'CONN':
67-
return {
68-
valid: false,
69-
error: 'SMTP server connection failed with error code: CONN',
70-
};
71-
default:
72-
break;
73-
}
74-
return {
75-
valid: false,
76-
error: JSON.stringify(e),
77-
};
51+
return connectionErrorHandler(e);
7852
}
7953
},
8054
});

‎packages/blocks/smtp/src/lib/connection-error-handler.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { logger } from '@openops/server-shared';
2+
3+
export function connectionErrorHandler(error: unknown): {
4+
valid: false;
5+
error: string;
6+
} {
7+
const code =
8+
typeof error === 'object' &&
9+
error !== null &&
10+
'code' in error &&
11+
typeof (error as { code?: unknown }).code === 'string'
12+
? (error as { code: string }).code
13+
: undefined;
14+
15+
logger.debug('Failed to validate SMTP connection', error);
16+
switch (code) {
17+
case 'EDNS':
18+
return {
19+
valid: false,
20+
error: 'SMTP server not found or unreachable. Error Code: EDNS',
21+
};
22+
case 'CONN':
23+
return {
24+
valid: false,
25+
error:
26+
'Could not establish a connection to the SMTP server. Error Code: CONN',
27+
};
28+
case 'ETIMEDOUT':
29+
return {
30+
valid: false,
31+
error: 'Connection to the SMTP server timed out. Error Code: ETIMEDOUT',
32+
};
33+
default:
34+
break;
35+
}
36+
37+
const errorMessage = code
38+
? `SMTP connection failed with error code: ${code}`
39+
: JSON.stringify(error);
40+
41+
return {
42+
valid: false,
43+
error: errorMessage,
44+
};
45+
}

‎packages/blocks/smtp/src/lib/get-smtp-ports.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { DropdownState } from '@openops/blocks-framework';
2+
import { SharedSystemProp, system } from '@openops/server-shared';
3+
4+
const getSMTPPorts = (): number[] | undefined => {
5+
const raw = system.getOrThrow(SharedSystemProp.SMTP_ALLOWED_PORTS);
6+
const ports = raw
7+
.split(',')
8+
.map((p) => p.trim())
9+
.filter((p) => p.length > 0)
10+
.map((p) => Number(p))
11+
.filter((port) => Number.isInteger(port) && port > 0 && port <= 65535);
12+
13+
if (ports.length === 0) {
14+
return undefined;
15+
}
16+
17+
return ports;
18+
};
19+
20+
export function getSmtpPortsOptions(): DropdownState<number> {
21+
const ports = getSMTPPorts();
22+
23+
return {
24+
disabled: !ports,
25+
error: ports
26+
? undefined
27+
: 'Invalid value for OPS_SMTP_ALLOWED_PORTS. At least one valid TCP port (1-65535) must be configured.',
28+
options: ports
29+
? ports.map((port) => {
30+
return {
31+
label: port.toString(),
32+
value: port,
33+
};
34+
})
35+
: [],
36+
};
37+
}

‎packages/blocks/smtp/test/connection-error-handler.test.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { connectionErrorHandler } from '../src/lib/connection-error-handler';
2+
3+
describe('connectionErrorHandler', () => {
4+
test('should return correct error message for EDNS error code', () => {
5+
const error = { code: 'EDNS' };
6+
const result = connectionErrorHandler(error);
7+
expect(result).toEqual({
8+
valid: false,
9+
error: 'SMTP server not found or unreachable. Error Code: EDNS',
10+
});
11+
});
12+
13+
test('should return correct error message for CONN error code', () => {
14+
const error = { code: 'CONN' };
15+
const result = connectionErrorHandler(error);
16+
expect(result).toEqual({
17+
valid: false,
18+
error:
19+
'Could not establish a connection to the SMTP server. Error Code: CONN',
20+
});
21+
});
22+
23+
test('should return correct error message for ETIMEDOUT error code', () => {
24+
const error = { code: 'ETIMEDOUT' };
25+
const result = connectionErrorHandler(error);
26+
expect(result).toEqual({
27+
valid: false,
28+
error: 'Connection to the SMTP server timed out. Error Code: ETIMEDOUT',
29+
});
30+
});
31+
32+
test('should return stringified error for error code not defined', () => {
33+
const error = { code: undefined };
34+
const result = connectionErrorHandler(error);
35+
expect(result).toEqual({
36+
valid: false,
37+
error: JSON.stringify(error),
38+
});
39+
});
40+
41+
test('should return default message with unknown error code', () => {
42+
const error = { code: 'UNKNOWN_CODE' };
43+
const result = connectionErrorHandler(error);
44+
expect(result).toEqual({
45+
valid: false,
46+
error: `SMTP connection failed with error code: UNKNOWN_CODE`,
47+
});
48+
});
49+
50+
test('should return stringified error when error object has no code', () => {
51+
const error = { message: 'Some error' };
52+
const result = connectionErrorHandler(error);
53+
expect(result).toEqual({
54+
valid: false,
55+
error: JSON.stringify(error),
56+
});
57+
});
58+
59+
test('should return stringified error for non-object errors', () => {
60+
const error = 'some string error';
61+
const result = connectionErrorHandler(error);
62+
expect(result).toEqual({
63+
valid: false,
64+
error: JSON.stringify(error),
65+
});
66+
});
67+
68+
test('should return stringified null for null error', () => {
69+
const error = null;
70+
const result = connectionErrorHandler(error);
71+
expect(result).toEqual({
72+
valid: false,
73+
error: JSON.stringify(error),
74+
});
75+
});
76+
});

‎packages/blocks/smtp/test/get-smtp-ports.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { SharedSystemProp, system } from '@openops/server-shared';
2+
import { getSmtpPortsOptions } from '../src/lib/get-smtp-ports';
3+
4+
jest.mock('@openops/server-shared', () => ({
5+
...jest.requireActual('@openops/server-shared'),
6+
system: {
7+
getOrThrow: jest.fn(),
8+
},
9+
}));
10+
11+
describe('getSmtpPortsOptions', () => {
12+
it('should return valid options when valid ports are provided', () => {
13+
(system.getOrThrow as jest.Mock).mockReturnValue('25, 465, 587');
14+
15+
const result = getSmtpPortsOptions();
16+
17+
expect(result).toEqual({
18+
disabled: false,
19+
error: undefined,
20+
options: [
21+
{ label: '25', value: 25 },
22+
{ label: '465', value: 465 },
23+
{ label: '587', value: 587 },
24+
],
25+
});
26+
expect(system.getOrThrow).toHaveBeenCalledWith(
27+
SharedSystemProp.SMTP_ALLOWED_PORTS,
28+
);
29+
});
30+
31+
it('should filter out invalid ports and return only valid ones', () => {
32+
(system.getOrThrow as jest.Mock).mockReturnValue(
33+
'25, abc, 0, 70000, 587, 25.5',
34+
);
35+
36+
const result = getSmtpPortsOptions();
37+
38+
expect(result.options).toEqual([
39+
{ label: '25', value: 25 },
40+
{ label: '587', value: 587 },
41+
]);
42+
});
43+
44+
it('should return disabled state and error message when no valid ports are provided', () => {
45+
(system.getOrThrow as jest.Mock).mockReturnValue('abc, -1, 0, 99999');
46+
47+
const result = getSmtpPortsOptions();
48+
49+
expect(result).toEqual({
50+
disabled: true,
51+
error:
52+
'Invalid value for OPS_SMTP_ALLOWED_PORTS. At least one valid TCP port (1-65535) must be configured.',
53+
options: [],
54+
});
55+
});
56+
57+
it('should return disabled state and error message when the input is empty or contains only separators', () => {
58+
(system.getOrThrow as jest.Mock).mockReturnValue(' , ,, ');
59+
60+
const result = getSmtpPortsOptions();
61+
62+
expect(result.disabled).toBe(true);
63+
expect(result.error).toBeDefined();
64+
expect(result.options).toEqual([]);
65+
});
66+
});

‎packages/server/shared/src/lib/system/system-prop.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ export enum SharedSystemProp {
159159
LANGFUSE_HOST = 'LANGFUSE_HOST',
160160

161161
ENABLE_HOST_VALIDATION = 'ENABLE_HOST_VALIDATION',
162+
SMTP_ALLOWED_PORTS = 'SMTP_ALLOWED_PORTS',
162163
}
163164

164165
export enum WorkerSystemProps {

‎packages/server/shared/src/lib/system/system.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ const systemPropDefaultValues: Partial<Record<SystemProp, string>> = {
109109
[AppSystemProp.TELEMETRY_COLLECTOR_URL]: 'https://telemetry.openops.com/save',
110110
[SharedSystemProp.ENABLE_HOST_VALIDATION]: 'true',
111111
[AppSystemProp.OPENOPS_ADMIN_PASSWORD_SALT]: '$2b$10$6zuoB5d8Dz9bzV91gpuynO',
112+
[SharedSystemProp.SMTP_ALLOWED_PORTS]: '25,465,587,2525',
112113
};
113114

114115
export const system = {

0 commit comments

Comments
 (0)