Fix: Prevent crash when parsing empty or malformed JSON response#5905
Fix: Prevent crash when parsing empty or malformed JSON response#5905TeamInfinixdevcom wants to merge 2 commits intoFlowiseAI:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's resilience by implementing a robust JSON parsing mechanism. It addresses potential crashes caused by malformed or empty JSON data in chat message fields, ensuring a more stable user experience by safely processing incoming data and preserving its original form when parsing is unsuccessful. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a safeJsonParse helper to prevent crashes when parsing potentially empty or malformed JSON strings in chat message fields, which is a good improvement. My review includes a high-severity bug fix for the new helper to correctly handle parsing of JSON null values, and a medium-severity refactoring suggestion to improve the maintainability of the parsing logic by reducing code repetition.
| const safeJsonParse = (input: string) => { | ||
| if (!input || input.trim() === '') return null | ||
| try { | ||
| return JSON.parse(input) | ||
| } catch (error) { | ||
| console.warn('Unable to parse chat message JSON field', error) | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
This safeJsonParse function has a subtle bug when parsing a valid JSON string that evaluates to null, such as 'null'. In this case, JSON.parse('null') correctly returns the value null. However, your function also returns null on parsing errors or for empty strings. At the call sites, you use safeJsonParse(...) ?? originalValue. When safeJsonParse returns null, this expression incorrectly falls back to originalValue.
To fix this, safeJsonParse should return undefined on failure. This allows distinguishing a failed parse from a successful parse that results in null.
safeJsonParse('null')would returnnull, andnull ?? 'null'correctly evaluates tonull.safeJsonParse('invalid')would returnundefined, andundefined ?? 'invalid'correctly evaluates to'invalid'.
| const safeJsonParse = (input: string) => { | |
| if (!input || input.trim() === '') return null | |
| try { | |
| return JSON.parse(input) | |
| } catch (error) { | |
| console.warn('Unable to parse chat message JSON field', error) | |
| return null | |
| } | |
| } | |
| const safeJsonParse = (input: string) => { | |
| if (!input || input.trim() === '') return undefined | |
| try { | |
| return JSON.parse(input) | |
| } catch (error) { | |
| console.warn('Unable to parse chat message JSON field', error) | |
| return undefined | |
| } | |
| } |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #336 by preventing crashes when chat message JSON-string fields contain empty/whitespace-only or malformed JSON, by introducing a safe parsing helper in the chat messages controller.
Changes:
- Added a
safeJsonParse()helper to guardJSON.parse()calls. - Updated chat message response parsing to use safe parsing and fall back to the original stored value on parse failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!input || input.trim() === '') return null | ||
| try { | ||
| return JSON.parse(input) | ||
| } catch (error) { | ||
| console.warn('Unable to parse chat message JSON field', error) | ||
| return null |
There was a problem hiding this comment.
safeJsonParse returns null on failure, and callers use ?? parsedResponse.<field>. This changes behavior for valid JSON that parses to null (e.g. the stored string 'null'): previously it would set the field to null, but now it falls back to the original string. Consider returning undefined (or a non-null sentinel) on parse failure/blank input so a successfully parsed null is preserved.
| if (!input || input.trim() === '') return null | |
| try { | |
| return JSON.parse(input) | |
| } catch (error) { | |
| console.warn('Unable to parse chat message JSON field', error) | |
| return null | |
| if (!input || input.trim() === '') return undefined | |
| try { | |
| return JSON.parse(input) | |
| } catch (error) { | |
| console.warn('Unable to parse chat message JSON field', error) | |
| return undefined |
| } catch (error) { | ||
| console.warn('Unable to parse chat message JSON field', error) | ||
| return null |
There was a problem hiding this comment.
The warning log in safeJsonParse doesn’t indicate which chat message field failed to parse (sourceDocuments vs usedTools vs artifacts, etc.), which will make production troubleshooting harder and may create noisy logs. Consider passing the field name into safeJsonParse (or logging it at the call site) and/or lowering the log level if malformed JSON is expected occasionally.
Fixes #336
This PR prevents the application from crashing when JSON.parse()
receives an empty or malformed string in chat message fields.
Changes:
This avoids the error:
SyntaxError: Unexpected end of JSON input