TykTechnologies / tyk

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

Merging to release-5.3.7: TT-13130 update gorpc version (#6644) #6647

Closed buger closed 1 month ago

buger commented 1 month 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 month ago

    API Changes

    no api changes detected
    github-actions[bot] commented 1 month 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. - Improve the RPC client connection handling.
    โฑ๏ธ Estimated effort to review: 2 ๐Ÿ”ต๐Ÿ”ตโšชโšชโšช
    ๐Ÿงช No relevant tests
    ๐Ÿ”’ No security concerns identified
    โšก Recommended focus areas for review

    Connection Handling
    Ensure that the new connection handling logic with WaitForConnection() is robust and handles all edge cases effectively.
    github-actions[bot] commented 1 month ago

    PR Code Suggestions โœจ

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add error handling for the WaitForConnection method to improve robustness ___ **Consider handling potential errors from clientSingleton.WaitForConnection() to
    ensure robust error management.** [rpc/rpc_client.go [309]](https://github.com/TykTechnologies/tyk/pull/6647/files#diff-3b88914c99bb9418e44e6389ce73579843562e8900730b380d7fff2e95c51033R309-R309) ```diff -clientSingleton.WaitForConnection() +if err := clientSingleton.WaitForConnection(); err != nil { + // handle error, possibly with logging or retry logic +} ```
    Suggestion importance[1-10]: 8 Why: The suggestion to handle potential errors from `clientSingleton.WaitForConnection()` is valuable as it addresses a possible bug by ensuring robust error management, which is crucial for maintaining the stability and reliability of the connection process.
    8
    sonarcloud[bot] commented 1 month ago

    Quality Gate Failed Quality Gate failed

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

    See analysis details on SonarCloud