Skip to content
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

feat(cli): Error message formatted #797

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

csehatt741
Copy link
Contributor

@csehatt741 csehatt741 commented Feb 25, 2025

User description

Description

Logging errors moved into the BaseCommand base class.

Fixes #795

Screenshots of relevant screens

image

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Enhancement


Description

  • Introduced a reusable logError method for error handling.

  • Replaced repetitive error logging/reporting with logError across commands.

  • Improved code maintainability and consistency in error handling.


Changes walkthrough 📝

Relevant files
Enhancement
20 files
base.command.ts
Added reusable `logError` method for error handling           
+19/-0   
create.environment.ts
Refactored error handling to use `logError`                           
+1/-4     
delete.environment.ts
Refactored error handling to use `logError`                           
+1/-4     
get.environment.ts
Refactored error handling to use `logError`                           
+1/-4     
list.environment.ts
Refactored error handling to use `logError`                           
+1/-4     
update.environment.ts
Refactored error handling to use `logError`                           
+1/-6     
create.project.ts
Refactored error handling to use `logError`                           
+1/-4     
delete.project.ts
Refactored error handling to use `logError`                           
+1/-4     
fork.project.ts
Refactored error handling to use `logError`                           
+1/-4     
get.project.ts
Refactored error handling to use `logError`                           
+1/-4     
list-forks.project.ts
Refactored error handling to use `logError`                           
+1/-6     
list.project.ts
Refactored error handling to use `logError`                           
+1/-4     
sync.project.ts
Refactored error handling to use `logError`                           
+1/-4     
unlink.project.ts
Refactored error handling to use `logError`                           
+1/-4     
update.project.ts
Refactored error handling to use `logError`                           
+1/-4     
create.secret.ts
Refactored error handling to use `logError`                           
+1/-4     
delete.secret.ts
Refactored error handling to use `logError`                           
+1/-4     
list.secret.ts
Refactored error handling to use `logError`                           
+1/-6     
revisions.secret.ts
Refactored error handling to use `logError`                           
+1/-4     
rollback.secret.ts
Refactored error handling to use `logError`                           
+1/-4     
Additional files
28 files
update.secret.ts +1/-4     
create.variable.ts +1/-4     
delete.variable.ts +1/-4     
list.variable.ts +1/-6     
revisions.variable.ts +1/-6     
rollback.variable.ts +1/-4     
update.variable.ts +1/-4     
create.workspace.ts +1/-4     
delete.workspace.ts +1/-4     
export.workspace.ts +1/-4     
get.workspace.ts +1/-4     
list.workspace.ts +1/-4     
accept-invitation.membership.ts +1/-4     
cancel-invitation.membership.ts +1/-4     
decline-invitation.membership.ts +1/-4     
get-all-members.membership.ts +1/-4     
invite.membership.ts +1/-4     
leave.membership.ts +1/-4     
remove.membership.ts +1/-4     
transfer-ownership.membership copy.ts +1/-4     
update-role.membership.ts +1/-4     
create.role.ts +1/-6     
delete.role.ts +1/-6     
get.role.ts +1/-6     
list.role.ts +1/-6     
update.role.ts +1/-6     
search.workspace.ts +1/-6     
update.workspace.ts +1/-4     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Parsing

    The logError method assumes the error message is always valid JSON with a body field. This could throw an exception if the error message is not in the expected format.

    const { body } = JSON.parse(error.message) as {
      body: string
    }
    Type Safety

    The error parameter type allows undefined values for message, error and statusCode fields which could lead to runtime errors when accessing these properties.

    error: { message: string; error: string; statusCode: number }

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Feb 25, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add JSON parse error handling
    Suggestion Impact:The commit implemented error handling for JSON.parse by wrapping it in a try-catch block, with error.message as fallback

    code diff:

    +    function getErrorMessage() {
    +      try {
    +        const { body } = JSON.parse(error.message) as {
    +          body: string
    +        }
    +        return body
    +      } catch {
    +        return error.message
    +      }

    Add error handling for the case when error.message is not a valid JSON string,
    as JSON.parse() could throw an exception.

    apps/cli/src/commands/base.command.ts [171-173]

    -const { body } = JSON.parse(error.message) as {
    -  body: string
    +let body: string;
    +try {
    +  body = (JSON.parse(error.message) as { body: string }).body;
    +} catch {
    +  body = error.message;
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    __

    Why: The suggestion addresses a critical error handling issue where JSON.parse could throw an exception if error.message is not valid JSON, which would crash the application. The fix provides a robust fallback.

    High
    • Update

    @csehatt741 csehatt741 changed the title featError message formatted feat(cli): Error message formatted Feb 25, 2025
    */
    protected logError(
    message: string,
    error: { message: string; error: string; statusCode: number }
    Copy link
    Member

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    instead of passing the message, you can use error.message.header

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    CLI: Update error message formatting
    2 participants