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

TT-13130 update gorpc version #6644

Closed sredxny closed 1 week ago

sredxny commented 1 week ago

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

    buger commented 1 week ago

    Knock Knock! ๐Ÿ”

    Just thought I'd let you know that your PR title and story title look quite different. PR titles that closely resemble the story title make it easier for reviewers to understand the context of the PR.

    An easy-to-understand PR title a day makes the reviewer review away! ๐Ÿ˜›โšก๏ธ
    Story Title Tyk Cloud: Panic appears when a user tried to deploy GW before Control Plane is in deployed state
    PR Title TT-13130 update gorpc version

    Check out this guide to learn more about PR best-practices.

    github-actions[bot] commented 1 week ago

    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

    API Change
    The method `WaitForConnection` is used instead of `ConnectionDialingWG.Wait`. Ensure that the new method covers all intended functionality and does not introduce any side effects.
    github-actions[bot] commented 1 week ago

    PR Code Suggestions โœจ

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Prevent potential nil pointer dereference by checking if clientSingleton is not nil before calling WaitForConnection() ___ **Replace the direct call to clientSingleton.WaitForConnection() with a conditional
    check to ensure clientSingleton is not nil before invoking WaitForConnection(). This
    prevents potential nil pointer dereference errors.** [rpc/rpc_client.go [308]](https://github.com/TykTechnologies/tyk/pull/6644/files#diff-3b88914c99bb9418e44e6389ce73579843562e8900730b380d7fff2e95c51033R308-R308) ```diff -clientSingleton.WaitForConnection() +if clientSingleton != nil { + clientSingleton.WaitForConnection() +} ```
    Suggestion importance[1-10]: 9 Why: The suggestion addresses a critical issue by preventing a potential nil pointer dereference, which could lead to runtime errors. Adding a nil check before calling `WaitForConnection()` ensures the code is more robust and prevents crashes.
    9
    github-actions[bot] commented 1 week ago

    API Changes

    no api changes detected
    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

    sredxny commented 1 week ago

    /release to release-5.3

    tykbot[bot] commented 1 week ago

    Working on it! Note that it can take a few minutes.

    ilijabojanovic commented 1 week ago

    /release to release-5.6.1

    tykbot[bot] commented 1 week ago

    @sredxny Succesfully merged PR

    tykbot[bot] commented 1 week ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 1 week ago

    @ilijabojanovic Succesfully merged PR

    sredxny commented 1 week ago

    /release to release-5.3.7

    tykbot[bot] commented 1 week ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 1 week ago

    @sredxny Succesfully merged PR

    sredxny commented 1 week ago

    /release to release-5.6

    tykbot[bot] commented 1 week ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 1 week ago

    @sredxny Succesfully merged PR

    sredxny commented 1 week ago

    /release to release-5-lts

    tykbot[bot] commented 1 week ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 1 week ago

    @sredxny Succesfully merged PR