-
Notifications
You must be signed in to change notification settings - Fork 32
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
PCH Smart Linking: Allow API retry when there are upstream errors #2386
Conversation
WalkthroughWalkthroughThe updates focus on enhancing the Smart Linking feature in the content helper tool by introducing a new error code Changes
Recent Review DetailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (2)
Files selected for processing (1)
Additional Context UsedPath-based Instructions (1)
Additional comments not posted (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 6
Review Status
Configuration used: .coderabbit.yaml
Files ignored due to path filters (11)
build/admin-settings.asset.php
is excluded by!build/**
build/admin-settings.css
is excluded by!build/**
build/content-helper/dashboard-widget.asset.php
is excluded by!build/**
build/content-helper/dashboard-widget.css
is excluded by!build/**
build/content-helper/dashboard-widget.js
is excluded by!build/**
build/content-helper/editor-sidebar.asset.php
is excluded by!build/**
build/content-helper/editor-sidebar.css
is excluded by!build/**
build/content-helper/editor-sidebar.js
is excluded by!build/**
build/content-helper/excerpt-generator.asset.php
is excluded by!build/**
build/content-helper/excerpt-generator.css
is excluded by!build/**
build/content-helper/excerpt-generator.js
is excluded by!build/**
Files selected for processing (7)
- src/content-helper/common/content-helper-error.tsx (2 hunks)
- src/content-helper/common/css/variables.scss (1 hunks)
- src/content-helper/editor-sidebar/smart-linking/component-block-overlay.tsx (5 hunks)
- src/content-helper/editor-sidebar/smart-linking/component.tsx (17 hunks)
- src/content-helper/editor-sidebar/smart-linking/provider.ts (2 hunks)
- src/content-helper/editor-sidebar/smart-linking/smart-linking.scss (2 hunks)
- src/content-helper/editor-sidebar/smart-linking/store.ts (7 hunks)
Additional Context Used
Path-based Instructions (7)
src/content-helper/common/css/variables.scss (1)
Pattern
**/*.{css,scss}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the SCSS code to ensure it is well-structured and adheres to best practices.
- Convert dimensions greater than or equal to 3px to rem units using the to_rem function.
- Utilize variables for sizes and colors defined in src/content-helper/common/css/variables.scss instead of hardcoding values."
src/content-helper/editor-sidebar/smart-linking/provider.ts (1)
Pattern
**/*.{js,ts,tsx,jsx}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/editor-sidebar/smart-linking/smart-linking.scss (1)
Pattern
**/*.{css,scss}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the SCSS code to ensure it is well-structured and adheres to best practices.
- Convert dimensions greater than or equal to 3px to rem units using the to_rem function.
- Utilize variables for sizes and colors defined in src/content-helper/common/css/variables.scss instead of hardcoding values."
src/content-helper/editor-sidebar/smart-linking/component-block-overlay.tsx (1)
Pattern
**/*.{js,ts,tsx,jsx}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/common/content-helper-error.tsx (1)
Pattern
**/*.{js,ts,tsx,jsx}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/editor-sidebar/smart-linking/store.ts (1)
Pattern
**/*.{js,ts,tsx,jsx}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/editor-sidebar/smart-linking/component.tsx (1)
Pattern
**/*.{js,ts,tsx,jsx}
: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
Additional comments not posted (12)
src/content-helper/editor-sidebar/smart-linking/provider.ts (2)
44-59
: Ensure proper cleanup ofAbortController
to prevent memory leaks.
Consider adding a destructor or cleanup method to handle the disposal ofAbortController
instances when theSmartLinkingProvider
is no longer in use.
89-99
: Validate the error handling for aborted requests to ensure it aligns with the application's overall error management strategy.
Consider centralizing error handling logic if similar patterns are used across different parts of the application.src/content-helper/editor-sidebar/smart-linking/component-block-overlay.tsx (2)
41-49
: Verify the use ofuseSelect
for managing state related to retrying. Ensure that this approach aligns with the overall state management strategy of the application.
133-150
: Consider enhancing the user experience by providing feedback or a countdown for the retry attempts.
This could help users understand how many attempts are left and potentially reduce confusion or frustration.src/content-helper/common/content-helper-error.tsx (1)
24-24
: Confirm the removal of OpenAI-related error codes aligns with the application's error handling strategy and that all necessary error scenarios are covered.src/content-helper/editor-sidebar/smart-linking/store.ts (1)
259-269
: Ensure that the retry attempt counter is reset appropriately in scenarios other than togglingisRetrying
. Consider edge cases where retries may be initiated multiple times within the same session.src/content-helper/editor-sidebar/smart-linking/component.tsx (6)
18-18
: Ensure theContentHelperErrorCode
import is used correctly throughout the file.
78-78
: TheMAX_NUMBER_OF_RETRIES
constant is well-defined and follows best practices.
116-117
: The addition ofretrying
andretryAttempt
states is appropriate for managing retry logic.
644-657
: ThegetGenerateButtonMessage
function correctly handles different states to provide user feedback.
681-682
: Ensure error messages are displayed correctly in the UI.Verification successful
The verification process has confirmed that
error.Message()
is indeed used within various.tsx
files in the codebase, including the specific file and context mentioned in the review comment (src/content-helper/editor-sidebar/smart-linking/component.tsx
). This indicates thaterror.Message()
is a valid method for displaying error messages in the UI across multiple components.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that error.Message() is a valid method and correctly displays error messages. rg --type tsx "error\.Message\(\)"Length of output: 64
Script:
#!/bin/bash # Re-verify that error.Message() is a valid method and correctly displays error messages by searching in .tsx files without specifying the file type. rg "error\.Message\(\)" --glob '*.tsx'Length of output: 567
709-714
: The button'sonClick
handler is correctly set to trigger thegenerateAndApplySmartLinks
function.
@coderabbitai resolve |
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.
Thanks for working on this, and for introducing the usage of an AbortController
. We should definitely use it in other parts of the PCH as well.
Description
In some scenarios, when generating Smart Links, the upstream might not be able to generate a valid response. This can happen because of issues communicating with OpenAI, or even invalid responses.
Since most of these issues are transient and can be resolved with a manual retry, this PR aims to implement a automatic retry process, up to three retries, to help overcoming these scenarios. This comes with a time cost though, since there can be up to three additional API requests.
The retries are communicated to the user by changing the label of the block overlay, and by changing the text in the "Adding Smart Links" button. We also give the option to abort, at any time, the retries attempts, in order to unblock the block editor - no pun intended.
Motivation and context
Improve the reliability of the Smart Linking feature, having it automatically handle any actionable failures.
How has this been tested?
Tested locally by editing the provider and simulating three API failures.
Screenshots (if appropriate)
Screen.Recording.2024-04-05.at.13.04.10.mov
Video: Smart Linking retry demonstration.
Summary by CodeRabbit
New Features
ParselyAborted
for enhanced error handling.Refactor
Style
Chores