fix: improve OpenRouter API key callback handling with better error handling and user feedback #6469
Conversation
…andling and user feedback - Add comprehensive logging to OpenRouter callback process for debugging - Show success/error messages to users when API key exchange completes - Ensure webview state updates after successful API key configuration - Add error handling for missing authorization codes in URI callback - Improve URI handler logging to help diagnose callback issues Fixes #6467
dosubot
bot
added
size:M
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| } else { | ||
| console.error(`[URI Handler] OpenRouter callback received without code parameter`) | ||
| vscode.window.showErrorMessage( | ||
| "OpenRouter authorization failed: No authorization code received. Please try again.", |
There was a problem hiding this comment.
User‐facing error messages (e.g. missing code) should use the translation function (t) rather than hardcoded strings.
| "OpenRouter authorization failed: No authorization code received. Please try again.", | |
| t("OpenRouter authorization failed: No authorization code received. Please try again."), |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| this.log(`[OpenRouter] Successfully updated provider profile with new API key`) | ||
|
|
||
| // Show success message to user | ||
| vscode.window.showInformationMessage("OpenRouter API key has been successfully configured!") |
There was a problem hiding this comment.
The success message shown to the user is hardcoded. Please use the t() function for localization.
| vscode.window.showInformationMessage("OpenRouter API key has been successfully configured!") | |
| vscode.window.showInformationMessage(t("openrouter:success.api_key_configured")) |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| ) | ||
|
|
||
| // Show user-friendly error message | ||
| vscode.window.showErrorMessage( |
There was a problem hiding this comment.
User‐facing error messages (e.g. when saving the API key fails) should use t() for translation instead of inline English strings.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for this comprehensive fix to the OpenRouter API key callback issue! The changes directly address the reported problem and add much-needed error handling and user feedback. I've identified some security and consistency concerns that should be addressed before merging.
|
|
||
| console.log(`[URI Handler] Received URI: ${uri.toString()}`) | ||
| console.log(`[URI Handler] Path: ${path}`) | ||
| console.log(`[URI Handler] Query params:`, Object.fromEntries(query.entries())) |
There was a problem hiding this comment.
Security concern: These logging statements expose potentially sensitive query parameters including authorization codes. Could we redact sensitive parameters like 'code' or use a more secure logging approach? Consider filtering out sensitive keys before logging.
|
|
||
| if (response.data && response.data.key) { | ||
| apiKey = response.data.key | ||
| this.log(`[OpenRouter] Successfully received API key: ${apiKey.substring(0, 10)}...`) |
There was a problem hiding this comment.
Security concern: Logging partial API keys could still be a security risk, especially in production environments. Is this intentional for debugging? Could we avoid logging API key fragments entirely or use a more secure approach like logging just the key length or a hash?
| } catch (error) { | ||
| console.error(`[URI Handler] Failed to handle OpenRouter callback:`, error) | ||
| // Error is already shown to user in handleOpenRouterCallback | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling: Here the error is caught but not re-thrown, while in ClineProvider.ts errors are re-thrown after showing user messages. Should these follow the same pattern for consistency? Consider whether the URI handler should also re-throw to maintain error propagation.
| console.error(`[URI Handler] OpenRouter callback received without code parameter`) | ||
| vscode.window.showErrorMessage( | ||
| "OpenRouter authorization failed: No authorization code received. Please try again.", | ||
| ) |
There was a problem hiding this comment.
Missing internationalization: This user-facing error message is hardcoded. Could this use the i18n system like other parts of the codebase? Consider using something like t('openrouter:errors.no_authorization_code').
|
|
||
| // Show user-friendly error message | ||
| vscode.window.showErrorMessage( | ||
| `Failed to get OpenRouter API key: ${error instanceof Error ? error.message : "Unknown error"}. Please try again.`, |
There was a problem hiding this comment.
Missing internationalization: These user-facing error messages are hardcoded. Could these use the i18n system for consistency with the rest of the codebase? This would also help with future localization efforts.
| this.log(`[OpenRouter] Successfully updated provider profile with new API key`) | ||
|
|
||
| // Show success message to user | ||
| vscode.window.showInformationMessage("OpenRouter API key has been successfully configured!") |
There was a problem hiding this comment.
Missing internationalization: Same concern here - this success message should use the i18n system for consistency and future localization support.
hannesrudolph
added
the
Issue/PR - Triage
|
Not a proper fix, closing |
This PR fixes issue #6467 where users reported that clicking the OpenRouter callback link after authorizing an API key would not populate the API key field in settings.
Changes Made
Enhanced Error Handling & User Feedback
Improved Callback Flow
postStateToWebview()to refresh the UIBetter User Experience
Testing
Root Cause Analysis
The issue was likely caused by:
This fix addresses all these issues by adding proper error handling, user feedback, and ensuring the UI updates correctly.
Fixes #6467
Important
Improves error handling and user feedback for OpenRouter API key callback in
handleUri.tsandClineProvider.ts, with enhanced logging and UI updates.handleUri.tsfor OpenRouter callback, logging errors and showing user messages for missing authorization codes.ClineProvider.ts,handleOpenRouterCallback()now logs API key exchange process and shows success/error messages to users.postStateToWebview()after API key configuration.handleUri.tsandClineProvider.tsfor debugging callback issues.This description was created by
for e7e258a. You can customize this summary. It will automatically update as commits are pushed.