TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.68k stars 1.08k forks source link

Merging to release-5.6: TT-13130 update gorpc version (#6644) #6648

Closed buger closed 1 week ago

buger commented 1 week ago

User description

TT-13130 update gorpc version (#6644)

User description

TT-13130
Summary Tyk Cloud: Panic appears when a user tried to deploy GW before Control Plane is in deployed state
Type Bug Bug
Status In Dev
Points N/A
Labels Re_open, QA_Fail

Description

Related Issue

TT-13130

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

Checklist


PR Type

Bug fix, Enhancement


Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
rpc_client.go
Update connection handling in RPC client                                 

rpc/rpc_client.go
  • Replaced ConnectionDialingWG.Wait() with WaitForConnection().
  • Improved connection handling logic.
  • +1/-1     
    Dependencies
    go.mod
    Update gorpc dependency version in go.mod                               

    go.mod - Updated `gorpc` dependency version.
    +1/-1     
    go.sum
    Update go.sum with new gorpc checksums                                     

    go.sum - Added new checksum entries for updated `gorpc` version.
    +2/-0     

    ๐Ÿ’ก PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information


    Co-authored-by: sredny buitrago sredny@srednys-MacBook-Pro.local


    PR Type

    Bug fix, Enhancement


    Description


    Changes walkthrough ๐Ÿ“

    Relevant files
    Enhancement
    rpc_client.go
    Update connection handling in RPC client                                 

    rpc/rpc_client.go
  • Replaced ConnectionDialingWG.Wait() with WaitForConnection().
  • Improved connection handling logic.
  • +1/-1     
    Dependencies
    go.mod
    Update gorpc dependency version in go.mod                               

    go.mod - Updated `gorpc` dependency version.
    +1/-1     
    go.sum
    Update go.sum with new gorpc checksums                                     

    go.sum - Added new checksum entries for updated `gorpc` version.
    +2/-2     

    ๐Ÿ’ก PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    github-actions[bot] commented 1 week ago

    API Changes

    no api changes detected
    github-actions[bot] commented 1 week ago

    PR Reviewer Guide ๐Ÿ”

    Here are some key observations to aid the review process:

    **๐ŸŽซ Ticket compliance analysis โœ…** **[6644](https://github.com/TykTechnologies/tyk/issues/6644) - Fully compliant** Fully compliant requirements: - Update the gorpc library to a newer version. - Modify the RPC client connection handling. Not compliant requirements: - There are no non-compliant requirements based on the provided ticket information.
    โฑ๏ธ Estimated effort to review: 2 ๐Ÿ”ต๐Ÿ”ตโšชโšชโšช
    ๐Ÿงช No relevant tests
    ๐Ÿ”’ No security concerns identified
    โšก No major issues detected
    github-actions[bot] commented 1 week ago

    PR Code Suggestions โœจ

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add error handling for the WaitForConnection method to manage potential connection errors ___ **Ensure that clientSingleton.WaitForConnection() properly handles any potential
    errors or exceptions that might arise during the connection wait process. Consider
    adding error handling logic after this method call to manage such scenarios
    effectively.** [rpc/rpc_client.go [309]](https://github.com/TykTechnologies/tyk/pull/6648/files#diff-3b88914c99bb9418e44e6389ce73579843562e8900730b380d7fff2e95c51033R309-R309) ```diff -clientSingleton.WaitForConnection() +if err := clientSingleton.WaitForConnection(); err != nil { + // handle error, possibly retry or log +} ```
    Suggestion importance[1-10]: 8 Why: The suggestion to add error handling for the `WaitForConnection` method is highly relevant, as it addresses potential issues that could arise during the connection process. Proper error handling can prevent unexpected failures and improve the robustness of the application.
    8
    sonarcloud[bot] commented 1 week ago

    Quality Gate Failed Quality Gate failed

    Failed conditions
    0.0% Coverage on New Code (required โ‰ฅ 80%)

    See analysis details on SonarCloud