From 36173a49488ebf1a96faaaba7cb287666f3615ce Mon Sep 17 00:00:00 2001 From: Ben Cooke Date: Thu, 29 Jan 2026 10:00:22 -0500 Subject: [PATCH] Use one timeout config for requests to translation providers (#34957) --- .../lib/src/server/default_config.ts | 9 +-- server/i18n/en.json | 16 +----- server/public/model/config.go | 55 ++----------------- server/public/model/config_test.go | 5 +- .../localization/auto_translation.tsx | 51 ++++++++++++++++- .../localization/localization.scss | 6 ++ webapp/channels/src/i18n/en.json | 3 + webapp/platform/types/src/config.ts | 7 +-- 8 files changed, 71 insertions(+), 81 deletions(-) diff --git a/e2e-tests/playwright/lib/src/server/default_config.ts b/e2e-tests/playwright/lib/src/server/default_config.ts index 9bbbcab53a1..6da4e0ba442 100644 --- a/e2e-tests/playwright/lib/src/server/default_config.ts +++ b/e2e-tests/playwright/lib/src/server/default_config.ts @@ -844,17 +844,12 @@ const defaultServerConfig: AdminConfig = { AutoTranslationSettings: { Enable: false, Provider: '', - TargetLanguages: ['en'], - TimeoutsMs: { - Short: 1200, - Medium: 2500, - Long: 6000, - Notification: 300, - }, LibreTranslate: { URL: '', APIKey: '', }, + TargetLanguages: ['en'], + TimeoutMs: 5000, Agents: { LLMServiceID: '', }, diff --git a/server/i18n/en.json b/server/i18n/en.json index f0d134ac93f..1ecff6174b9 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -9905,20 +9905,8 @@ "translation": "Unsupported autotranslation provider." }, { - "id": "model.config.is_valid.autotranslation.timeouts.long.app_error", - "translation": "Invalid long timeout for autotranslation settings. Must be a positive number." - }, - { - "id": "model.config.is_valid.autotranslation.timeouts.medium.app_error", - "translation": "Invalid medium timeout for autotranslation settings. Must be a positive number." - }, - { - "id": "model.config.is_valid.autotranslation.timeouts.notification.app_error", - "translation": "Invalid notification timeout for autotranslation settings. Must be a positive number." - }, - { - "id": "model.config.is_valid.autotranslation.timeouts.short.app_error", - "translation": "Invalid short timeout for autotranslation settings. Must be a positive number." + "id": "model.config.is_valid.autotranslation.timeout.app_error", + "translation": "Invalid timeout for autotranslation settings. Must be a positive number." }, { "id": "model.config.is_valid.cache_type.app_error", diff --git a/server/public/model/config.go b/server/public/model/config.go index 367e2127f82..a66d0834c2e 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -2787,24 +2787,11 @@ type AutoTranslationSettings struct { Enable *bool `access:"site_localization,cloud_restrictable"` Provider *string `access:"site_localization,cloud_restrictable"` TargetLanguages *[]string `access:"site_localization,cloud_restrictable"` - TimeoutsMs *AutoTranslationTimeoutsInMs `access:"site_localization,cloud_restrictable"` + TimeoutMs *int `access:"site_localization,cloud_restrictable"` LibreTranslate *LibreTranslateProviderSettings `access:"site_localization,cloud_restrictable"` Agents *AgentsProviderSettings `access:"site_localization,cloud_restrictable"` } -// AutoTranslationTimeoutsInMs defines content-aware timeout thresholds. -// Based on LibreTranslate benchmark findings, timeouts are set according to content length: -// - Short: ≤200 runes -// - Medium: ≤500 runes -// - Long: >500 runes -// - Notification: preserved for notification-specific timeout requirements -type AutoTranslationTimeoutsInMs struct { - Short *int `access:"site_localization,cloud_restrictable"` // ≤200 runes, default: 1200ms - Medium *int `access:"site_localization,cloud_restrictable"` // ≤500 runes, default: 2500ms - Long *int `access:"site_localization,cloud_restrictable"` // >500 runes, default: 6000ms - Notification *int `access:"site_localization,cloud_restrictable"` // Notification timeout, default: 300ms -} - // LibreTranslateProviderSettings configures the LibreTranslate translation provider. type LibreTranslateProviderSettings struct { URL *string `access:"site_localization,cloud_restrictable"` // LibreTranslate server URL @@ -2828,10 +2815,9 @@ func (s *AutoTranslationSettings) SetDefaults() { s.TargetLanguages = &[]string{"en"} } - if s.TimeoutsMs == nil { - s.TimeoutsMs = &AutoTranslationTimeoutsInMs{} + if s.TimeoutMs == nil { + s.TimeoutMs = NewPointer(5000) } - s.TimeoutsMs.SetDefaults() if s.LibreTranslate == nil { s.LibreTranslate = &LibreTranslateProviderSettings{} @@ -2844,24 +2830,6 @@ func (s *AutoTranslationSettings) SetDefaults() { s.Agents.SetDefaults() } -func (s *AutoTranslationTimeoutsInMs) SetDefaults() { - if s.Short == nil { - s.Short = NewPointer(1200) - } - - if s.Medium == nil { - s.Medium = NewPointer(2500) - } - - if s.Long == nil { - s.Long = NewPointer(6000) - } - - if s.Notification == nil { - s.Notification = NewPointer(300) - } -} - func (s *LibreTranslateProviderSettings) SetDefaults() { if s.URL == nil { s.URL = NewPointer("") @@ -4850,20 +4818,9 @@ func (s *AutoTranslationSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.provider.unsupported.app_error", nil, "", http.StatusBadRequest) } - // Validate timeouts if set - if s.TimeoutsMs != nil { - if s.TimeoutsMs.Short != nil && *s.TimeoutsMs.Short <= 0 { - return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.timeouts.short.app_error", nil, "", http.StatusBadRequest) - } - if s.TimeoutsMs.Medium != nil && *s.TimeoutsMs.Medium <= 0 { - return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.timeouts.medium.app_error", nil, "", http.StatusBadRequest) - } - if s.TimeoutsMs.Long != nil && *s.TimeoutsMs.Long <= 0 { - return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.timeouts.long.app_error", nil, "", http.StatusBadRequest) - } - if s.TimeoutsMs.Notification != nil && *s.TimeoutsMs.Notification <= 0 { - return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.timeouts.notification.app_error", nil, "", http.StatusBadRequest) - } + // Validate timeout if set (must be positive) + if s.TimeoutMs != nil && *s.TimeoutMs <= 0 { + return NewAppError("Config.IsValid", "model.config.is_valid.autotranslation.timeout.app_error", nil, "", http.StatusBadRequest) } return nil diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 6c4c4b9eb21..9bf79a6e16c 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -2649,10 +2649,7 @@ func TestAutoTranslationSettingsDefaults(t *testing.T) { require.False(t, *c.AutoTranslationSettings.Enable) require.Equal(t, "", *c.AutoTranslationSettings.Provider) - require.Equal(t, 1200, *c.AutoTranslationSettings.TimeoutsMs.Short) - require.Equal(t, 2500, *c.AutoTranslationSettings.TimeoutsMs.Medium) - require.Equal(t, 6000, *c.AutoTranslationSettings.TimeoutsMs.Long) - require.Equal(t, 300, *c.AutoTranslationSettings.TimeoutsMs.Notification) + require.Equal(t, 5000, *c.AutoTranslationSettings.TimeoutMs) require.Equal(t, "", *c.AutoTranslationSettings.LibreTranslate.URL) require.Equal(t, "", *c.AutoTranslationSettings.LibreTranslate.APIKey) // TODO: Enable Agents provider in future release diff --git a/webapp/channels/src/components/admin_console/localization/auto_translation.tsx b/webapp/channels/src/components/admin_console/localization/auto_translation.tsx index c1bd47ba29e..d5e85233af5 100644 --- a/webapp/channels/src/components/admin_console/localization/auto_translation.tsx +++ b/webapp/channels/src/components/admin_console/localization/auto_translation.tsx @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import React, {useCallback, useMemo, useState} from 'react'; -import {defineMessages, FormattedMessage} from 'react-intl'; +import {defineMessage, defineMessages, FormattedMessage} from 'react-intl'; import {Link} from 'react-router-dom'; import type {AutoTranslationSettings} from '@mattermost/types/config'; @@ -14,6 +14,7 @@ import { SectionContent, SectionHeader, } from 'components/admin_console/system_properties/controls'; +import TextSetting from 'components/admin_console/text_setting'; import useGetAgentsBridgeEnabled from 'components/common/hooks/useGetAgentsBridgeEnabled'; import Toggle from 'components/toggle'; @@ -52,6 +53,12 @@ export default function AutoTranslation(props: SystemConsoleCustomSettingsCompon return settings; }); + // Track timeout input separately to allow intermediate invalid states while typing + const [timeoutInputValue, setTimeoutInputValue] = useState( + String(autoTranslationSettings.TimeoutMs || 5000), + ); + const [timeoutError, setTimeoutError] = useState(''); + const handleChange = useCallback((id: string, value: AutoTranslationSettings[keyof AutoTranslationSettings] | string[]) => { const updatedSettings = { ...autoTranslationSettings, @@ -61,6 +68,22 @@ export default function AutoTranslation(props: SystemConsoleCustomSettingsCompon props.onChange(props.id, updatedSettings); }, [props, autoTranslationSettings]); + const handleTimeoutChange = useCallback((id: string, value: string) => { + setTimeoutInputValue(value); + + const numValue = parseInt(value, 10); + if (value === '' || isNaN(numValue) || numValue <= 0) { + setTimeoutError('Timeout must be a positive number'); + + // Propagate 0 so backend validation will reject the save + handleChange(id, 0); + return; + } + + setTimeoutError(''); + handleChange(id, numValue); + }, [handleChange]); + const handleToggle = useCallback(() => { const newValue = !autoTranslationSettings.Enable; const newSettings = { @@ -234,6 +257,32 @@ export default function AutoTranslation(props: SystemConsoleCustomSettingsCompon disabled={props.disabled || props.setByEnv} setByEnv={props.setByEnv} /> + + } + placeholder={defineMessage({ + id: 'admin.site.localization.autoTranslationTimeoutPlaceholder', + defaultMessage: 'e.g.: 5000', + })} + helpText={timeoutError ? ( + {timeoutError} + ) : ( + + )} + type='number' + value={timeoutInputValue} + setByEnv={props.setByEnv} + onChange={handleTimeoutChange} + disabled={props.disabled} + /> } diff --git a/webapp/channels/src/components/admin_console/localization/localization.scss b/webapp/channels/src/components/admin_console/localization/localization.scss index a9f2466c6c7..c84b4e62eff 100644 --- a/webapp/channels/src/components/admin_console/localization/localization.scss +++ b/webapp/channels/src/components/admin_console/localization/localization.scss @@ -91,3 +91,9 @@ } } +.autotranslation-error.error-message { + color: var(--error-text); + font-size: 12px; + font-weight: 400; +} + diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 1b10118cab0..ea5ba8d172d 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -2863,6 +2863,9 @@ "admin.site.localization.autoTranslationProviderLibreTranslateURLExample": "e.g.: \"https://libretranslate.yourdomain.com\"", "admin.site.localization.autoTranslationProviderLibreTranslateURLTitle": "LibreTranslate API Endpoint:", "admin.site.localization.autoTranslationProviderTitle": "Translation provider", + "admin.site.localization.autoTranslationTimeoutDescription": "Maximum time in milliseconds to wait for a translation response. Default is 5000ms (5 seconds).", + "admin.site.localization.autoTranslationTimeoutPlaceholder": "e.g.: 5000", + "admin.site.localization.autoTranslationTimeoutTitle": "Translation timeout (ms):", "admin.site.localization.enableAutoTranslationDescription": "Configure auto-translation for channels and direct messages", "admin.site.localization.enableAutoTranslationTitle": "Auto-translation", "admin.site.localization.goToAgentsConfig": "Go to Agents plugin config", diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index 1a61a8d7918..12013157dcd 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -763,12 +763,7 @@ export type AutoTranslationSettings = { Agents?: { LLMServiceID: string; }; - TimeoutsMs: { - Short: number; - Medium: number; - Long: number; - Notification: number; - }; + TimeoutMs: number; }; export type SamlSettings = {