fix: prevent client-side crashes from null/undefined plugin data in workflow (#23154) (#23182)

This commit is contained in:
lyzno1
2025-07-31 10:03:33 +08:00
committed by GitHub
parent 646900b00c
commit 1b2046da3f
9 changed files with 260 additions and 14 deletions

View File

@@ -49,9 +49,9 @@ describe('check-i18n script functionality', () => {
}
vm.runInNewContext(transpile(content), context)
const translationObj = moduleExports.default || moduleExports
const translationObj = (context.module.exports as any).default || context.module.exports
if(!translationObj || typeof translationObj !== 'object')
if (!translationObj || typeof translationObj !== 'object')
throw new Error(`Error parsing file: ${filePath}`)
const nestedKeys: string[] = []
@@ -62,7 +62,7 @@ describe('check-i18n script functionality', () => {
// This is an object (but not array), recurse into it but don't add it as a key
iterateKeys(obj[key], nestedKey)
}
else {
else {
// This is a leaf node (string, number, boolean, array, etc.), add it as a key
nestedKeys.push(nestedKey)
}
@@ -73,7 +73,7 @@ describe('check-i18n script functionality', () => {
const fileKeys = nestedKeys.map(key => `${camelCaseFileName}.${key}`)
allKeys.push(...fileKeys)
}
catch (error) {
catch (error) {
reject(error)
}
})
@@ -272,9 +272,6 @@ export default translation
const filteredEnKeys = allEnKeys.filter(key =>
key.startsWith(targetFile.replace(/[-_](.)/g, (_, c) => c.toUpperCase())),
)
const filteredZhKeys = allZhKeys.filter(key =>
key.startsWith(targetFile.replace(/[-_](.)/g, (_, c) => c.toUpperCase())),
)
expect(allEnKeys).toHaveLength(4) // 2 keys from each file
expect(filteredEnKeys).toHaveLength(2) // only components keys

View File

@@ -0,0 +1,207 @@
/**
* Test cases to reproduce the plugin tool workflow error
* Issue: #23154 - Application error when loading plugin tools in workflow
* Root cause: split() operation called on null/undefined values
*/
describe('Plugin Tool Workflow Error Reproduction', () => {
/**
* Mock function to simulate the problematic code in switch-plugin-version.tsx:29
* const [pluginId] = uniqueIdentifier.split(':')
*/
const mockSwitchPluginVersionLogic = (uniqueIdentifier: string | null | undefined) => {
// This directly reproduces the problematic line from switch-plugin-version.tsx:29
const [pluginId] = uniqueIdentifier!.split(':')
return pluginId
}
/**
* Test case 1: Simulate null uniqueIdentifier
* This should reproduce the error mentioned in the issue
*/
it('should reproduce error when uniqueIdentifier is null', () => {
expect(() => {
mockSwitchPluginVersionLogic(null)
}).toThrow('Cannot read properties of null (reading \'split\')')
})
/**
* Test case 2: Simulate undefined uniqueIdentifier
*/
it('should reproduce error when uniqueIdentifier is undefined', () => {
expect(() => {
mockSwitchPluginVersionLogic(undefined)
}).toThrow('Cannot read properties of undefined (reading \'split\')')
})
/**
* Test case 3: Simulate empty string uniqueIdentifier
*/
it('should handle empty string uniqueIdentifier', () => {
expect(() => {
const result = mockSwitchPluginVersionLogic('')
expect(result).toBe('') // Empty string split by ':' returns ['']
}).not.toThrow()
})
/**
* Test case 4: Simulate malformed uniqueIdentifier without colon separator
*/
it('should handle malformed uniqueIdentifier without colon separator', () => {
expect(() => {
const result = mockSwitchPluginVersionLogic('malformed-identifier-without-colon')
expect(result).toBe('malformed-identifier-without-colon') // No colon means full string returned
}).not.toThrow()
})
/**
* Test case 5: Simulate valid uniqueIdentifier
*/
it('should work correctly with valid uniqueIdentifier', () => {
expect(() => {
const result = mockSwitchPluginVersionLogic('valid-plugin-id:1.0.0')
expect(result).toBe('valid-plugin-id')
}).not.toThrow()
})
})
/**
* Test for the variable processing split error in use-single-run-form-params
*/
describe('Variable Processing Split Error', () => {
/**
* Mock function to simulate the problematic code in use-single-run-form-params.ts:91
* const getDependentVars = () => {
* return varInputs.map(item => item.variable.slice(1, -1).split('.'))
* }
*/
const mockGetDependentVars = (varInputs: Array<{ variable: string | null | undefined }>) => {
return varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0) // Filter out empty arrays
}
/**
* Test case 1: Variable processing with null variable
*/
it('should handle null variable safely', () => {
const varInputs = [{ variable: null }]
expect(() => {
mockGetDependentVars(varInputs)
}).not.toThrow()
const result = mockGetDependentVars(varInputs)
expect(result).toEqual([]) // null variables are filtered out
})
/**
* Test case 2: Variable processing with undefined variable
*/
it('should handle undefined variable safely', () => {
const varInputs = [{ variable: undefined }]
expect(() => {
mockGetDependentVars(varInputs)
}).not.toThrow()
const result = mockGetDependentVars(varInputs)
expect(result).toEqual([]) // undefined variables are filtered out
})
/**
* Test case 3: Variable processing with empty string
*/
it('should handle empty string variable', () => {
const varInputs = [{ variable: '' }]
expect(() => {
mockGetDependentVars(varInputs)
}).not.toThrow()
const result = mockGetDependentVars(varInputs)
expect(result).toEqual([]) // Empty string is filtered out, so result is empty array
})
/**
* Test case 4: Variable processing with valid variable format
*/
it('should work correctly with valid variable format', () => {
const varInputs = [{ variable: '{{workflow.node.output}}' }]
expect(() => {
mockGetDependentVars(varInputs)
}).not.toThrow()
const result = mockGetDependentVars(varInputs)
expect(result[0]).toEqual(['{workflow', 'node', 'output}'])
})
})
/**
* Integration test to simulate the complete workflow scenario
*/
describe('Plugin Tool Workflow Integration', () => {
/**
* Simulate the scenario where plugin metadata is incomplete or corrupted
* This can happen when:
* 1. Plugin is being loaded from marketplace but metadata request fails
* 2. Plugin configuration is corrupted in database
* 3. Network issues during plugin loading
*/
it('should reproduce the client-side exception scenario', () => {
// Mock incomplete plugin data that could cause the error
const incompletePluginData = {
// Missing or null uniqueIdentifier
uniqueIdentifier: null,
meta: null,
minimum_dify_version: undefined,
}
// This simulates the error path that leads to the white screen
expect(() => {
// Simulate the code path in switch-plugin-version.tsx:29
// The actual problematic code doesn't use optional chaining
const _pluginId = (incompletePluginData.uniqueIdentifier as any).split(':')[0]
}).toThrow('Cannot read properties of null (reading \'split\')')
})
/**
* Test the scenario mentioned in the issue where plugin tools are loaded in workflow
*/
it('should simulate plugin tool loading in workflow context', () => {
// Mock the workflow context where plugin tools are being loaded
const workflowPluginTools = [
{
provider_name: 'test-plugin',
uniqueIdentifier: null, // This is the problematic case
tool_name: 'test-tool',
},
{
provider_name: 'valid-plugin',
uniqueIdentifier: 'valid-plugin:1.0.0',
tool_name: 'valid-tool',
},
]
// Process each plugin tool
workflowPluginTools.forEach((tool, _index) => {
if (tool.uniqueIdentifier === null) {
// This reproduces the exact error scenario
expect(() => {
const _pluginId = (tool.uniqueIdentifier as any).split(':')[0]
}).toThrow()
}
else {
// Valid tools should work fine
expect(() => {
const _pluginId = tool.uniqueIdentifier.split(':')[0]
}).not.toThrow()
}
})
})
})

View File

@@ -26,7 +26,8 @@ export type SwitchPluginVersionProps = {
export const SwitchPluginVersion: FC<SwitchPluginVersionProps> = (props) => {
const { uniqueIdentifier, tooltip, onChange, className } = props
const [pluginId] = uniqueIdentifier.split(':')
const [pluginId] = uniqueIdentifier?.split(':') || ['']
const [isShow, setIsShow] = useState(false)
const [isShowUpdateModal, { setTrue: showUpdateModal, setFalse: hideUpdateModal }] = useBoolean(false)
const [target, setTarget] = useState<{
@@ -60,6 +61,11 @@ export const SwitchPluginVersion: FC<SwitchPluginVersionProps> = (props) => {
})
}
const { t } = useTranslation()
// Guard against null/undefined uniqueIdentifier to prevent app crash
if (!uniqueIdentifier || !pluginId)
return null
return <Tooltip popupContent={!isShow && !isShowUpdateModal && tooltip} triggerMethod='hover'>
<div className={cn('flex w-fit items-center justify-center', className)} onClick={e => e.stopPropagation()}>
{isShowUpdateModal && pluginDetail && <PluginMutationModel

View File

@@ -77,7 +77,13 @@ const useSingleRunFormParams = ({
}, [runResult, t])
const getDependentVars = () => {
return varInputs.map(item => item.variable.slice(1, -1).split('.'))
return varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
}
return {

View File

@@ -62,7 +62,13 @@ const useSingleRunFormParams = ({
}, [inputVarValues, setInputVarValues, varInputs])
const getDependentVars = () => {
return varInputs.map(item => item.variable.slice(1, -1).split('.'))
return varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
}
return {

View File

@@ -168,7 +168,13 @@ const useSingleRunFormParams = ({
})()
const getDependentVars = () => {
const promptVars = varInputs.map(item => item.variable.slice(1, -1).split('.'))
const promptVars = varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
const contextVar = payload.context.variable_selector
const vars = [...promptVars, contextVar]
if (isVisionModel && payload.vision?.enabled && payload.vision?.configs?.variable_selector) {

View File

@@ -120,7 +120,13 @@ const useSingleRunFormParams = ({
})()
const getDependentVars = () => {
const promptVars = varInputs.map(item => item.variable.slice(1, -1).split('.'))
const promptVars = varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
const vars = [payload.query, ...promptVars]
if (isVisionModel && payload.vision?.enabled && payload.vision?.configs?.variable_selector) {
const visionVar = payload.vision.configs.variable_selector

View File

@@ -118,7 +118,13 @@ const useSingleRunFormParams = ({
})()
const getDependentVars = () => {
const promptVars = varInputs.map(item => item.variable.slice(1, -1).split('.'))
const promptVars = varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
const vars = [payload.query_variable_selector, ...promptVars]
if (isVisionModel && payload.vision?.enabled && payload.vision?.configs?.variable_selector) {
const visionVar = payload.vision.configs.variable_selector

View File

@@ -88,7 +88,13 @@ const useSingleRunFormParams = ({
const toolIcon = useToolIcon(payload)
const getDependentVars = () => {
return varInputs.map(item => item.variable.slice(1, -1).split('.'))
return varInputs.map((item) => {
// Guard against null/undefined variable to prevent app crash
if (!item.variable || typeof item.variable !== 'string')
return []
return item.variable.slice(1, -1).split('.')
}).filter(arr => arr.length > 0)
}
return {