Skip to content
This repository was archived by the owner on Jun 3, 2026. It is now read-only.

fix: surface MaxTokensError when max_tokens truncates tool input JSON #1065

Open
cmbullock wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: surface MaxTokensError when max_tokens truncates tool input JSON #1065

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e3df3c
fix: surface MaxTokensError when max_tokens truncates tool input JSON
cmbullock May 13, 2026
646316e
Address feedback
cmbullock May 20, 2026
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
28 changes: 25 additions & 3 deletions strands-ts/src/models/__tests__/model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,23 +323,45 @@ describe('Model', () => {
)
})

it('preserves SyntaxError instead of overwriting with MaxTokensError when tool input JSON is malformed', async () => {
it('throws MaxTokenError when contentBlockStop arrives with truncated tool input JSON and stopReason is maxTokens', async () => {

@JackYPCOnline JackYPCOnline Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: typo in the test description — MaxTokenError should be MaxTokensError (plural, matching the class in errors.ts). The assertion itself is correct; just the it(...) string.

const provider = new TestModelProvider(async function* () {
yield { type: 'modelMessageStartEvent', role: 'assistant' }
yield {
type: 'modelContentBlockStartEvent',
start: { type: 'toolUseStart', toolUseId: 'tool1', name: 'get_weather' },
start: { type: 'toolUseStart', toolUseId: 't', name: 'tool' },
}
yield {
type: 'modelContentBlockDeltaEvent',
delta: { type: 'toolUseInputDelta', input: '{invalid json' },
delta: { type: 'toolUseInputDelta', input: '{"field": "value"' },
}
yield { type: 'modelContentBlockStopEvent' }
yield { type: 'modelMessageStopEvent', stopReason: 'maxTokens' }
})

const messages = [new Message({ role: 'user', content: [new TextBlock('Hi')] })]

await expect(async () => await collectGenerator(provider.streamAggregated(messages))).rejects.toThrow(
MaxTokensError
)
})

@github-actions github-actions Bot May 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Issue: Missing test coverage for the path where the stream ends without a modelMessageStopEvent while deferredToolInputParseError is set (i.e., the 'Stream ended without completing a message' error with the SyntaxError cause attached at line 477).

Suggestion: Add a test case where the generator yields a tool input delta + contentBlockStopEvent (triggering the parse error) but does NOT yield a modelMessageStopEvent. This exercises the new cause attachment on the "stream ended" error path.

@cmbullock cmbullock May 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Test added


it('surfaces SyntaxError as cause when tool input JSON is malformed and stopReason is not maxTokens', async () => {
const provider = new TestModelProvider(async function* () {
yield { type: 'modelMessageStartEvent', role: 'assistant' }
yield {
type: 'modelContentBlockStartEvent',
start: { type: 'toolUseStart', toolUseId: 't', name: 'tool' },
}
yield {
type: 'modelContentBlockDeltaEvent',
delta: { type: 'toolUseInputDelta', input: '{invalid json' },
}
yield { type: 'modelContentBlockStopEvent' }
yield { type: 'modelMessageStopEvent', stopReason: 'toolUse' }
})

const messages = [new Message({ role: 'user', content: [new TextBlock('Hi')] })]

try {
await collectGenerator(provider.streamAggregated(messages))
expect.fail('Expected error to be thrown')
Expand Down
14 changes: 11 additions & 3 deletions strands-ts/src/models/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export abstract class Model<T extends BaseModelConfig = BaseModelConfig> {
let finalStopReason: StopReason | null = null
let metadata: ModelMetadataEvent | undefined = undefined
let redactionMessage: string | undefined = undefined
let deferredToolInputParseError: SyntaxError | undefined = undefined

for await (const event_data of this.stream(messages, options)) {
const event = this._convert_to_class_event(event_data)
Expand Down Expand Up @@ -425,8 +426,8 @@ export abstract class Model<T extends BaseModelConfig = BaseModelConfig> {
yield block
} catch (e: unknown) {
if (e instanceof SyntaxError) {
logger.error('unable to parse JSON string', e)
throw e
logger.error('unable to parse tool input JSON', e)
deferredToolInputParseError = e
}
}
break
Expand Down Expand Up @@ -471,7 +472,10 @@ export abstract class Model<T extends BaseModelConfig = BaseModelConfig> {

if (!stoppedMessage || !finalStopReason) {
// If we exit the loop without completing a message or stop reason, throw an error
throw new ModelError('Stream ended without completing a message')
throw new ModelError(
'Stream ended without completing a message',
deferredToolInputParseError ? { cause: deferredToolInputParseError } : undefined
)
}

// Attach metadata after redaction so it applies to the final message.
Expand All @@ -495,6 +499,10 @@ export abstract class Model<T extends BaseModelConfig = BaseModelConfig> {
)
}

if (deferredToolInputParseError !== undefined) {
throw deferredToolInputParseError

@github-actions github-actions Bot May 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Issue: When stopReason is not maxTokens, the deferred SyntaxError is thrown raw and relies on the outer catch block to wrap it in a ModelError. This produces a generic message like "Expected ',' or '}' in JSON at position X" rather than something actionable.

Suggestion: Consider wrapping it explicitly to preserve the diagnostic context already in the log message:

if (deferredToolInputParseError !== undefined) {
  throw new ModelError('unable to parse tool input JSON', { cause: deferredToolInputParseError })
}

This makes the error message self-descriptive and avoids depending on the outer catch's generic wrapping behavior.

@cmbullock cmbullock May 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Error explicitly wrapped and test updated

}

// Return the final message with stop reason and optional metadata
const result: StreamAggregatedResult = {
message: stoppedMessage,
Expand Down
Loading