-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add proper error handling for API conversation history issues #4312
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.
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
Add proper error handling for API conversation history issues #4312
Conversation
Add detailed logging to readApiMessages function to help diagnose "Unexpected: No existing API conversation history" errors. The logs will show: - When the history file exists but contains an empty array - When the old history file exists but contains an empty array - When neither history file is found This will help diagnose issues like the one in #4311 where the API conversation history file gets truncated to an empty array during task resumption with orchestrator tasks. Fixes: #4311 Signed-off-by: Eric Wheeler <[email protected]>
Add error handling for JSON parsing failures in readApiMessages: - Wrap JSON.parse calls in try/catch blocks for both current and legacy history files - Add detailed error logging with taskId and file paths - Return empty arrays when parsing fails to maintain function contract - Improve existing debug logging for empty history arrays - Handle file unlinking even when parsing fails This prevents crashes from malformed JSON in history files and provides better debugging information when API conversation history is corrupted. Related to issue #4311. Signed-off-by: Eric Wheeler <[email protected]>
21a8167 to
9d19c68
Compare
|
See notes on the issue as to why I moved this to draft |
|
I just wanted to provide the steps I took for troubleshooting and case it helps someone else along the way. These debug prints are non-intrusive, uncommon, only trigger upon error, and only serve to help troubleshoot this problem if somebody else hits it. IMHO, PRs like this should be merged quickly to help future troubleshooting by others. (Someday if I clean up and delete the branch, then this research could be lost.) |
|
@cte - Windows test error is probably unrelated to this pull request - do you know what is going on there? |
|
Looking at this PR, I noticed the debug logging is only written to console.error. Have you considered propagating these error details up to the actual error that users see? Currently, when Wouldn't it be more helpful if the error message included the specific reason? For example:
This way users would immediately know what went wrong without needing to check developer console logs. The debug info could either be:
What do you think about enhancing the error propagation in addition to the console logging? |
It is a good idea, however I do not really want to put a lot more work into this. Is there a VS Code function that can replace console.error to provide a simple pop-up? if there is, then probably all The intention of this PR was just to provide a little more information to possibly help others by simple debug; in this case the review process is introducing unnecessary complexity to what was intended as trivial. Sometimes when troubleshooting an issue if I do not find a complete solution, I will create a pull request with whatever instrumentation led me there, in hopes that someone else can report with additional information. The error framework that you suggest as a great idea, and maybe it should be taken on by another developer in a separate pull request to streamline PRs like this in the future. If you (or someone else) would like to implement such a thing then please do, but I think if this pull request is not acceptable in its current state than we should close it because I do not have the bandwidth to take that on at the moment. |
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, this will help us understand what might be going on when tasks disappear in some cases.
…eInc#4312) Co-authored-by: Eric Wheeler <[email protected]>
Co-authored-by: Eric Wheeler <[email protected]>
Context
This PR adds detailed debug logging to the
readApiMessagesfunction to help diagnose "Unexpected: No existing API conversation history" errors.Implementation
Added logging for three scenarios:
How to Test
When encountering the "Unexpected: No existing API conversation history" error, check the Developer Tools console (Help > Toggle Developer Tools > Console tab) for debug messages with the format:
[Roo-Debug] readApiMessages: ...These logs will show the taskId and file path, helping to diagnose the issue.
Screenshots
N/A - Console logging only
Get in Touch
Discord: KJ7LNW
does not completely fix #4311 but it may provide debug output that may help us fix it from other users.
Important
Adds detailed debug logging to
readApiMessagesinapiMessages.tsfor diagnosing issues with API conversation history files.readApiMessagesinapiMessages.tsfor diagnosing missing or empty API conversation history files.This description was created by
for 9d19c68. You can customize this summary. It will automatically update as commits are pushed.