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

fix: Error messages fixed in api-key service #418

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

Nil2000
Copy link
Contributor

@Nil2000 Nil2000 commented Sep 10, 2024

User description

Description

Error messages updated in keyshade/apps/api/src/api-key/service/api-key.service.ts file

Fixes #417

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

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

Bug fix


Description

  • Updated error messages in the ApiKeyService to provide clearer and more accurate information by using apiKeySlug instead of apiKeyId.
  • Ensured consistency in error handling across methods in the ApiKeyService.

Changes walkthrough 📝

Relevant files
Bug fix
api-key.service.ts
Fix error messages in API key service                                       

apps/api/src/api-key/service/api-key.service.ts

  • Updated error messages for NotFoundException.
  • Changed error message to use apiKeySlug instead of apiKeyId.
  • +3/-3     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Inconsistent Error Handling
    The error message in the catch block (line 140) throws a NotFoundException, which may not be appropriate for all types of errors that could occur during the delete operation.

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Sep 10, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    ✅ Reorder code to prevent potential null reference errors

    Consider moving the apiKey check before accessing apiKey.id to avoid potential
    runtime errors if apiKey is null.

    apps/api/src/api-key/service/api-key.service.ts [88-97]

     const apiKey = await this.prisma.apiKey.findUnique({
       where: {
         slug: apiKeySlug
       }
     })
    -const apiKeyId = apiKey.id
     
     if (!apiKey) {
       throw new NotFoundException(`API key ${apiKeySlug} not found`)
     }
     
    +const apiKeyId = apiKey.id
    +

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential bug where accessing apiKey.id could result in a runtime error if apiKey is null. Reordering the code to check for null before accessing properties is a crucial fix.

    9
    Error handling
    Improve error handling to distinguish between different types of errors

    Consider adding more specific error handling in the deleteApiKey method to
    distinguish between "not found" errors and other potential issues.

    apps/api/src/api-key/service/api-key.service.ts [132-141]

     try {
       await this.prisma.apiKey.delete({
         where: {
           slug: apiKeySlug,
           userId: user.id
         }
       })
     } catch (error) {
    -  throw new NotFoundException(`API key ${apiKeySlug} not found`)
    +  if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') {
    +    throw new NotFoundException(`API key ${apiKeySlug} not found`)
    +  }
    +  throw error
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves error handling by distinguishing between different types of errors, which enhances the robustness and clarity of the code.

    8
    Consistency
    Standardize error handling approach across different methods

    Consider using a consistent approach for error handling across all methods. The
    getApiKeyBySlug method doesn't use a try-catch block, unlike deleteApiKey.

    apps/api/src/api-key/service/api-key.service.ts [155-167]

    -const apiKey = await this.prisma.apiKey.findUnique({
    -  where: {
    -    slug: apiKeySlug,
    -    userId: user.id
    -  },
    -  select: this.apiKeySelect
    -})
    +try {
    +  const apiKey = await this.prisma.apiKey.findUnique({
    +    where: {
    +      slug: apiKeySlug,
    +      userId: user.id
    +    },
    +    select: this.apiKeySelect
    +  })
     
    -if (!apiKey) {
    -  throw new NotFoundException(`API key ${apiKeySlug} not found`)
    +  if (!apiKey) {
    +    throw new NotFoundException(`API key ${apiKeySlug} not found`)
    +  }
    +
    +  return apiKey
    +} catch (error) {
    +  if (error instanceof NotFoundException) {
    +    throw error
    +  }
    +  throw new InternalServerErrorException('An error occurred while fetching the API key')
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Standardizing error handling across methods can improve code consistency and maintainability, though it may not be as critical as addressing potential runtime errors.

    7

    @rajdip-b rajdip-b merged commit edfbce0 into keyshade-xyz:develop Sep 10, 2024
    6 checks passed
    rajdip-b pushed a commit that referenced this pull request Sep 16, 2024
    ## [2.5.0](v2.4.0...v2.5.0) (2024-09-16)
    
    ### 🚀 Features
    
    * **api-client:** Added workspace controller ([#427](#427)) ([2f4edec](2f4edec))
    * **api-client:** Added workspace role controller ([#430](#430)) ([b03ce8e](b03ce8e))
    * **api-client:** Synced with latest API ([27f4309](27f4309))
    * **api:** Add slug in entities ([#415](#415)) ([89e2fcc](89e2fcc))
    * **api:** Included default workspace details in getSelf function ([#414](#414)) ([e67bbd6](e67bbd6))
    * **platform:** Add loading skeleton in the [secure]s page ([#423](#423)) ([a97681e](a97681e))
    * **schema:** Added a schema package ([01ea232](01ea232))
    * **web:** Update about and careers page ([e167f53](e167f53))
    
    ### 🐛 Bug Fixes
    
    * **api:** Error messages fixed in api-key service ([#418](#418)) ([edfbce0](edfbce0))
    
    ### 📚 Documentation
    
    * Fixed minor typo in postman workspace link ([#411](#411)) ([ed23116](ed23116))
    * Updated Postman links ([444bfb1](444bfb1))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Suppressed version check test in [secure] ([4688e8c](4688e8c))
    * **api:** Update slug generation method ([#420](#420)) ([1f864df](1f864df))
    
    ### 🔨 Code Refactoring
    
    * **API:** Refactor workspace-membership into a separate module ([#421](#421)) ([574170f](574170f))
    * **platform:** added optional chaining due to strict null check ([#413](#413)) ([907e369](907e369))
    @rajdip-b
    Copy link
    Member

    🎉 This PR is included in version 2.5.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    Kiranchaudhary537 pushed a commit to Kiranchaudhary537/keyshade that referenced this pull request Oct 13, 2024
    ## [2.5.0](keyshade-xyz/keyshade@v2.4.0...v2.5.0) (2024-09-16)
    
    ### 🚀 Features
    
    * **api-client:** Added workspace controller ([keyshade-xyz#427](keyshade-xyz#427)) ([2f4edec](keyshade-xyz@2f4edec))
    * **api-client:** Added workspace role controller ([keyshade-xyz#430](keyshade-xyz#430)) ([b03ce8e](keyshade-xyz@b03ce8e))
    * **api-client:** Synced with latest API ([27f4309](keyshade-xyz@27f4309))
    * **api:** Add slug in entities ([keyshade-xyz#415](keyshade-xyz#415)) ([89e2fcc](keyshade-xyz@89e2fcc))
    * **api:** Included default workspace details in getSelf function ([keyshade-xyz#414](keyshade-xyz#414)) ([e67bbd6](keyshade-xyz@e67bbd6))
    * **platform:** Add loading skeleton in the [secure]s page ([keyshade-xyz#423](keyshade-xyz#423)) ([a97681e](keyshade-xyz@a97681e))
    * **schema:** Added a schema package ([01ea232](keyshade-xyz@01ea232))
    * **web:** Update about and careers page ([e167f53](keyshade-xyz@e167f53))
    
    ### 🐛 Bug Fixes
    
    * **api:** Error messages fixed in api-key service ([keyshade-xyz#418](keyshade-xyz#418)) ([edfbce0](keyshade-xyz@edfbce0))
    
    ### 📚 Documentation
    
    * Fixed minor typo in postman workspace link ([keyshade-xyz#411](keyshade-xyz#411)) ([ed23116](keyshade-xyz@ed23116))
    * Updated Postman links ([444bfb1](keyshade-xyz@444bfb1))
    
    ### 🔧 Miscellaneous Chores
    
    * **api:** Suppressed version check test in [secure] ([4688e8c](keyshade-xyz@4688e8c))
    * **api:** Update slug generation method ([keyshade-xyz#420](keyshade-xyz#420)) ([1f864df](keyshade-xyz@1f864df))
    
    ### 🔨 Code Refactoring
    
    * **API:** Refactor workspace-membership into a separate module ([keyshade-xyz#421](keyshade-xyz#421)) ([574170f](keyshade-xyz@574170f))
    * **platform:** added optional chaining due to strict null check ([keyshade-xyz#413](keyshade-xyz#413)) ([907e369](keyshade-xyz@907e369))
    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.

    API: Update error messages in api-key service
    2 participants