fix: clean up two explicit errors handled elsewhere #138
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for hardfunai canceled.
|
adamrturman
commented
Jan 15, 2025
Comment on lines
-161
to
-165
| // Errors can be thrown when recursive call is cancelled | ||
| if (errorMessage.includes('JSON')) { | ||
| errorMessage = 'You clicked cancel - please try again'; | ||
| } | ||
|
|
There was a problem hiding this comment.
No longer an error after introducing JSON streaming parser.
adamrturman
commented
Jan 15, 2025
Comment on lines
-65
to
-73
| const lastMessage = messages[messages.length - 1]; | ||
| const lastMessageRole = lastMessage.role; | ||
|
|
||
| // if a user has initiated a wallet-based tool call, but not confirmed or denied it | ||
| // we don't want to add more messages to the history | ||
| const userRequestInProgress = | ||
| messages.length > 1 && lastMessageRole === 'user'; | ||
|
|
||
| if (isRequesting || userRequestInProgress) { |
There was a problem hiding this comment.
We now early return from the streaming store if we get into a bad state and don't call the .write() method which triggered the error when a user entered more messages after starting a tx, but never completing that tool call in metamask.
oros/src/toolCallStreamStore.ts
Lines 64 to 66 in 1d3f3c4
There was a problem hiding this comment.
the reason for the write error in the streaming store is because:
- model generates a single tool call which gets added to the toolCallStream store the index for the tool call is 0
- the tool call stream store parses the streaming json for the tool call and removes the underlying parser once the tool call stream is finished from the model.
- another tool call is made but it's scoped to a different request, however the problem is that the index of the new tool call corresponds with the old tool call which had it's parser removed, so when the new tool call json args are being streamed this is when the errors gets thrown. The problem here is that the old tool call from the previous request is not removed/resolved yet, and there's no way for us to know that this reffers to a different request all together since all we get is an index for each tool call request from the model. and those indices are scoped per request. so the problem here is that we shouldn't allow users to submit more requests until the previous tool call is resolved/removed in some way.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Removes two explicit error checks