Skip to content

fix(executor): guard against missing content in provider responses #3952

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
lawrence3699 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(executor): guard against missing content in provider responses #3952

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
23 changes: 23 additions & 0 deletions apps/sim/executor/handlers/evaluator/evaluator-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,29 @@ describe('EvaluatorBlockHandler', () => {
expect((result as any).fluency).toBe(0)
})

it('should throw a clear error when provider returns no content', async () => {
const inputs = {
content: 'Test content to evaluate',
metrics: [{ name: 'quality', description: 'Quality', range: { min: 0, max: 10 } }],
apiKey: 'test-api-key',
}

mockFetch.mockImplementationOnce(() => {
return Promise.resolve({
ok: true,
json: () =>
Promise.resolve({
model: 'gpt-4o',
tokens: { input: 50, output: 0, total: 50 },
}),
})
})

await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
'Provider returned an empty response'
)
})

it('should extract metric scores ignoring case', async () => {
const inputs = {
content: 'Test',
Expand Down
6 changes: 6 additions & 0 deletions apps/sim/executor/handlers/evaluator/evaluator-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,12 @@ export class EvaluatorBlockHandler implements BlockHandler {

const result = await response.json()

if (!result.content) {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}

const parsedContent = this.extractJSONFromResponse(result.content)
Comment on lines 146 to 154
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!result.content) won’t catch truthy non-string content, so extractJSONFromResponse(result.content) can still receive a non-string and crash when it calls .trim(). Also, whitespace-only strings slip through and will likely produce a confusing downstream parse/score behavior. Consider validating typeof result.content === 'string' and result.content.trim() before proceeding.

Copilot uses AI. Check for mistakes.

const metricScores = this.extractMetricScores(parsedContent, inputs.metrics)
Expand Down
65 changes: 65 additions & 0 deletions apps/sim/executor/handlers/router/router-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,45 @@ describe('RouterBlockHandler', () => {
await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow('Server error')
})

it('should throw a clear error when provider returns no content', async () => {
const inputs = { prompt: 'Choose the best option.', apiKey: 'test-api-key' }

mockFetch.mockImplementationOnce(() => {
return Promise.resolve({
ok: true,
json: () =>
Promise.resolve({
model: 'gpt-4o',
tokens: { input: 100, output: 0, total: 100 },
}),
})
})

await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
'Provider returned an empty response'
)
})

it('should throw a clear error when provider content is empty string', async () => {
const inputs = { prompt: 'Choose the best option.', apiKey: 'test-api-key' }

mockFetch.mockImplementationOnce(() => {
return Promise.resolve({
ok: true,
json: () =>
Promise.resolve({
content: '',
model: 'gpt-4o',
tokens: { input: 100, output: 0, total: 100 },
}),
})
})

await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow(
'Provider returned an empty response'
)
})

it('should handle Azure OpenAI models with endpoint and API version', async () => {
const inputs = {
prompt: 'Choose the best option.',
Expand Down Expand Up @@ -587,4 +626,30 @@ describe('RouterBlockHandler V2', () => {
expect(result.selectedRoute).toBe('route-1')
expect(result.reasoning).toBe('')
})

it('should throw a clear error when provider returns no content (V2)', async () => {
const inputs = {
context: 'I need help with billing',
model: 'gpt-4o',
apiKey: 'test-api-key',
routes: JSON.stringify([
{ id: 'route-support', title: 'Support', value: 'Customer support inquiries' },
]),
}

mockFetch.mockImplementationOnce(() => {
return Promise.resolve({
ok: true,
json: () =>
Promise.resolve({
model: 'gpt-4o',
tokens: { input: 150, output: 0, total: 150 },
}),
})
})

await expect(handler.execute(mockContext, mockRouterV2Block, inputs)).rejects.toThrow(
'Provider returned an empty response'
)
})
})
12 changes: 12 additions & 0 deletions apps/sim/executor/handlers/router/router-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ export class RouterBlockHandler implements BlockHandler {

const result = await response.json()

if (!result.content) {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}

const chosenBlockId = result.content.trim().toLowerCase()
Comment on lines +127 to 133
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard if (!result.content) only catches missing/empty values, but it will still allow truthy non-string content (e.g., an object/array) through and then result.content.trim() will throw. It also treats whitespace-only strings as non-empty, leading to a later Invalid routing decision: error instead of the intended clear provider-empty error. Consider validating result is an object and typeof result.content === 'string' and that result.content.trim().length > 0 before calling .trim()/.toLowerCase().

Suggested change
if (!result.content) {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}
const chosenBlockId = result.content.trim().toLowerCase()
if (!result || typeof result !== 'object' || typeof result.content !== 'string') {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}
const normalizedContent = result.content.trim()
if (normalizedContent.length === 0) {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}
const chosenBlockId = normalizedContent.toLowerCase()

Copilot uses AI. Check for mistakes.
const chosenBlock = targetBlocks?.find((b) => b.id === chosenBlockId)

Expand Down Expand Up @@ -273,6 +279,12 @@ export class RouterBlockHandler implements BlockHandler {

const result = await response.json()

if (!result.content) {
throw new Error(
'Provider returned an empty response. The model may be unavailable or the request was malformed.'
)
}

Comment on lines 280 to +287
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the legacy path: if (!result.content) doesn’t protect against truthy non-string values, and whitespace-only content will fall through to the JSON parse / fallback and eventually call .trim() anyway. Tighten the validation to require a non-empty trimmed string (and ideally guard result itself) before using result.content.

Copilot uses AI. Check for mistakes.
let chosenRouteId: string
let reasoning = ''

Expand Down
Loading