Skip to content
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

fix(langchain): Fix structured parser error handling #7232

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(langchain): Fix structured parser error handling #7232

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
22 changes: 15 additions & 7 deletions langchain/src/output_parsers/structured.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,23 @@ ${JSON.stringify(zodToJsonSchema(this.schema))}
const json = text.includes("```")
? text.trim().split(/```(?:json)?/)[1]
: text.trim();

return await this.schema.parseAsync(JSON.parse(json));
} catch (e) {
try {
return await this.schema.parseAsync(JSON.parse(text.trim()));
} catch (e2) {
throw new OutputParserException(
`Failed to parse. Text: "${text}". Error: ${e2}`,
text
);
// eslint-disable-next-line no-instanceof/no-instanceof
if (e instanceof SyntaxError && e.message.includes("JSON")) {
Copy link
Member

Choose a reason for hiding this comment

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

Can't use instanceof as it's not available in all JS environments.

Copy link
Collaborator

@jacoblee93 jacoblee93 Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's not that it doesn't exist, it's just that it's not always reliable for packages since in some environments bundlers can duplicate class definitions across multiple chunks

(native classes I think are ok)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would be better to change this condition to something like if (e && (e as Error).name !== "ZodError")?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately minifiers can also mess with that 😬

It looks like SyntaxError is a native object though?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SyntaxError

So probably ok to do the instanceof check

// In case the error is JSON.parse related, try to parse the text directly
try {
return await this.schema.parseAsync(JSON.parse(text.trim()));
} catch (e2) {
throw new OutputParserException(
`Failed to parse. Text: "${text}". Error: ${e2}`,
Copy link
Member

Choose a reason for hiding this comment

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

If e2 is an object, it will not be rendered properly in this message. Please add a check beforehand which stringifies the error if it's an object

This was an issue before, but we should fix now.

text
);
}
} else {
// Otherwise, throw the original error (e.g. ZodError)
throw e;
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions langchain/src/output_parsers/tests/structured.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,31 @@ test("StructuredOutputParser throws error for JSON with backticks both inside an

await expect(parser.parse(text)).rejects.toThrow("Failed to parse");
});

test("StructuredOutputParser handles valid JSON without triple backticks not conforming to schema", async () => {
const parser = StructuredOutputParser.fromZodSchema(
z.object({
name: z.string().describe("Human name"),
age: z.number().describe("Human age"),
})
);
const text = '{"name": "John Doe", "age": null}';

await expect(parser.parse(text)).rejects.toThrow(
"Expected number, received null"
);
});

test("StructuredOutputParser handles valid JSON with backticks and not conforming to schema", async () => {
const parser = StructuredOutputParser.fromZodSchema(
z.object({
name: z.string().describe("Human name"),
age: z.number().describe("Human age"),
})
);
const text = '```json\n{"name": "John Doe", "age": null }```';

await expect(parser.parse(text)).rejects.toThrow(
"Expected number, received null"
);
});
Loading