-
Notifications
You must be signed in to change notification settings - Fork 0
release/v5.3.1 #45
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
release/v5.3.1 #45
Changes from all commits
31543d0
7bc424a
989d7d3
0f5483d
7a4d721
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,9 @@ | ||||||||||||||||||||||||||||||||||||||
| import React, { useState, useCallback, memo, useRef, useEffect } from "react" | ||||||||||||||||||||||||||||||||||||||
| import { useTranslation } from "react-i18next" | ||||||||||||||||||||||||||||||||||||||
| import { VSCodeButton } from "@vscode/webview-ui-toolkit/react" | ||||||||||||||||||||||||||||||||||||||
| import { XCircle, Info } from "lucide-react" | ||||||||||||||||||||||||||||||||||||||
| import { useCopyToClipboard } from "@src/utils/clipboard" | ||||||||||||||||||||||||||||||||||||||
| import { vscode } from "@src/utils/vscode" | ||||||||||||||||||||||||||||||||||||||
| import { VSCodeButton } from "@vscode/webview-ui-toolkit/react" | ||||||||||||||||||||||||||||||||||||||
| import { Info } from "lucide-react" | ||||||||||||||||||||||||||||||||||||||
| import React, { memo, useCallback, useEffect, useRef, useState } from "react" | ||||||||||||||||||||||||||||||||||||||
| import { useTranslation } from "react-i18next" | ||||||||||||||||||||||||||||||||||||||
| import CodeBlock from "../kilocode/common/CodeBlock" // kilocode_change | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| export interface ErrorRowProps { | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -125,7 +126,6 @@ export const ErrorRow = memo( | |||||||||||||||||||||||||||||||||||||
| }`} | ||||||||||||||||||||||||||||||||||||||
| onClick={handleToggleExpand}> | ||||||||||||||||||||||||||||||||||||||
| <div className="flex items-center gap-2 flex-grow"> | ||||||||||||||||||||||||||||||||||||||
| <XCircle className="w-3 h-3 text-vscode-foreground opacity-50" /> | ||||||||||||||||||||||||||||||||||||||
| <span className="font-bold">{errorTitle}</span> | ||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||
| <div className="flex items-center"> | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -154,11 +154,8 @@ export const ErrorRow = memo( | |||||||||||||||||||||||||||||||||||||
| <div className="relative my-1"> | ||||||||||||||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||||||||||||||
| className={ | ||||||||||||||||||||||||||||||||||||||
| headerClassName || "flex items-center gap-2 py-1.5 px-2 rounded-md bg-vscode-editor-background" | ||||||||||||||||||||||||||||||||||||||
| headerClassName || "flex items-center gap-2 py-2 px-2 rounded-md bg-vscode-editor-background" | ||||||||||||||||||||||||||||||||||||||
| }> | ||||||||||||||||||||||||||||||||||||||
| {/* Error Icon */} | ||||||||||||||||||||||||||||||||||||||
| <XCircle className="w-4 h-4 flex-shrink-0 text-vscode-foreground opacity-50" /> | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| {/* Error Title */} | ||||||||||||||||||||||||||||||||||||||
| {errorTitle && ( | ||||||||||||||||||||||||||||||||||||||
| <span className="text-vscode-editor-foreground font-medium text-sm whitespace-nowrap"> | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -219,6 +216,25 @@ export const ErrorRow = memo( | |||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| {/* Retry Button - outside tooltip, below error row */} | ||||||||||||||||||||||||||||||||||||||
| {type === "api_failure" && message?.includes("Provider error:") && ( | ||||||||||||||||||||||||||||||||||||||
| <div className="mt-1 flex justify-start"> | ||||||||||||||||||||||||||||||||||||||
| <VSCodeButton | ||||||||||||||||||||||||||||||||||||||
| appearance="secondary" | ||||||||||||||||||||||||||||||||||||||
| className="p-0" | ||||||||||||||||||||||||||||||||||||||
| onClick={() => { | ||||||||||||||||||||||||||||||||||||||
| // This will be handled by the parent component | ||||||||||||||||||||||||||||||||||||||
| vscode.postMessage({ | ||||||||||||||||||||||||||||||||||||||
| type: "askResponse", | ||||||||||||||||||||||||||||||||||||||
| askResponse: "yesButtonClicked", | ||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||
| }}> | ||||||||||||||||||||||||||||||||||||||
| <span className="codicon codicon-refresh mr-1" /> | ||||||||||||||||||||||||||||||||||||||
| {t("chat:retry.title")} | ||||||||||||||||||||||||||||||||||||||
| </VSCodeButton> | ||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+220
to
+236
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Functional Issue Issue: The Retry button sends an Fix: Remove the retry button from Impact: Prevents confusing UI elements that do not work.
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| <style>{` | ||||||||||||||||||||||||||||||||||||||
| @keyframes fadeIn { | ||||||||||||||||||||||||||||||||||||||
| from { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
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.
🟡 Type Safety
Issue:
chunk.erroris cast to{ code?: number }, but OpenAI/OpenRouter API error codes are typically strings (e.g., "invalid_api_key"). Assigning a string toerr.status(which implies an HTTP status code) is misleading and violates the type assertion.Fix: Update the type assertion to allow string codes and only assign to
statusif it is a number.Impact: Prevents runtime type mismatches and potential logic errors downstream.
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.
/matter nothing will break existing right?
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.
No, this change won't break existing functionality! The fix is fully backward compatible:
✅ Safe Changes:
✅ What Actually Changes:
err.statuswill now only be set for numeric HTTP status codes (proper behavior)status✅ Example Impact:
"invalid_api_key"→err.status = "invalid_api_key"(wrong!)"invalid_api_key"→err.status = undefined(correct!)401→err.status = 401(correct!)401→err.status = 401(still correct!)The change actually makes error handling more predictable and prevents downstream code from making incorrect assumptions about error types. Your existing error handling logic will work the same or better!