Upgrading status cmd to show connected peers ICE candidates #107
Conversation
WalkthroughThe recent updates introduce the functionality to display ICE candidate pairs for connected peers in the status command. This involves modifications across several files to handle peer statistics, enhance error handling, and improve the overall user experience with color-coded outputs and more detailed peer status information. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant CLI
participant Peer
participant StatusHandler
User->>CLI: Run status command
CLI->>StatusHandler: Request peer stats
StatusHandler->>Peer: Retrieve ICE candidate pairs
Peer-->>StatusHandler: Return ICE candidate pairs
StatusHandler-->>CLI: Send formatted status message
CLI-->>User: Display status with ICE candidate pairs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
✅ Deploy Preview for spectacular-biscochitos-00a738 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
webexec.go (1)
Line range hint
427-489: Significant changes inTrickleCandidatesto handle candidate trickling with error handling and POST/PUT requests.This function is quite complex and handles multiple responsibilities. Consider breaking it down into smaller functions for better maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- peers/pane.go (1 hunks)
- sock.go (4 hunks)
- webexec.go (9 hunks)
Additional context used
LanguageTool
CHANGELOG.md
[grammar] ~7-~7: Consider using either the past participle “defined” or the present participle “defining” here.
Context: ...pec/v2.0.0.html). his file's format is define in [Keep a Changelog](https://keepacha...
[grammar] ~16-~16: There seems to be a noun/verb agreement error. Did you mean “shows” or “showed”?
Context: ...leased ### Added - the status command show the chosen ICE candidate pairs for conn...
[grammar] ~104-~104: The word ‘install’ is not a noun.
Context: ... ## [1.0.1] 2023-8-3 ### Fixed - The install script ## [1.0.0] 2023-7-31 ### Fixed...
[grammar] ~182-~182: The word ‘install’ is not a noun.
Context: ....9] 2022-10-06 ### Fixed - updted the install script which was stuck on 0.17.6 ### [...
[grammar] ~213-~213: Did you mean Apple’s computer “Mac” (= trademark, capitalized)?
Context: ...staller messages are much improved on a mac ## [0.17.4] 2022-08-13 ### Fixed - r...
[grammar] ~245-~245: The operating system from Apple is written “macOS”.
Context: ... - Handling of PeerBook disconnection - MacOS one line installer ## [0.16.0] 2022-01...
[duplication] ~366-~366: Possible typo: you repeated a word
Context: ...ed to add their key ### Fixed - crash on on closing an already closed peerbook conn...
[uncategorized] ~403-~403: Do not mix variants of the same word (‘signalling’ and ‘signaling’) within a single text.
Context: ...xed - passing candidates regardless of signalling state (which is unreliable) ## [0.10.1...
[grammar] ~537-~537: There seems to be a noun/verb agreement error. Did you mean “errs” or “erred”?
Context: ....8.1] - 2020-2-4 ### Added - redirect err to an error file - Added pion logging c...
[uncategorized] ~611-~611: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...ow ## [0.5.4] - 2020-1-5 ### Added - Auto releasing with wide os & architecture support ##...
[uncategorized] ~652-~652: This expression is usually spelled with a hyphen.
Context: ... to store output - Orderly shutdown and marker based restore ## [0.4.3] - 2020-11-24 ### F...
[uncategorized] ~683-~683: This expression is usually spelled with a hyphen.
Context: ... ## [0.4.0] - 2020-10-14 ### Fixed - Linter based code beutification ### Added - Screen...
[grammar] ~692-~692: This phrase is duplicated. You should probably use “sub commands” only once.
Context: ...t runs in the backgroung and managed by sub commands - Sub commands - help, auth, start, stop, status, rest...
Markdownlint
CHANGELOG.md
3-3: Expected: 0 or 2; Actual: 1
Trailing spaces
7-7: Expected: 0 or 2; Actual: 1
Trailing spaces
68-68: Expected: 0 or 2; Actual: 1
Trailing spaces
70-70: Expected: 0 or 2; Actual: 1
Trailing spaces
102-102: Expected: 0 or 2; Actual: 1
Trailing spaces
130-130: Expected: 0 or 2; Actual: 1
Trailing spaces
139-139: Expected: 0 or 2; Actual: 1
Trailing spaces
174-174: Expected: 0 or 2; Actual: 1
Trailing spaces
224-224: Expected: 0 or 2; Actual: 1
Trailing spaces
230-230: Expected: 0 or 2; Actual: 1
Trailing spaces
236-236: Expected: 0 or 2; Actual: 1
Trailing spaces
240-240: Expected: 0 or 2; Actual: 1
Trailing spaces
280-280: Expected: 0 or 2; Actual: 1
Trailing spaces
282-282: Expected: 0 or 2; Actual: 1
Trailing spaces
318-318: Expected: 0 or 2; Actual: 1
Trailing spaces
338-338: Expected: 0 or 2; Actual: 1
Trailing spaces
342-342: Expected: 0 or 2; Actual: 1
Trailing spaces
345-345: Expected: 0 or 2; Actual: 1
Trailing spaces
373-373: Expected: 0 or 2; Actual: 1
Trailing spaces
379-379: Expected: 0 or 2; Actual: 1
Trailing spaces
388-388: Expected: 0 or 2; Actual: 1
Trailing spaces
391-391: Expected: 0 or 2; Actual: 1
Trailing spaces
413-413: Expected: 0 or 2; Actual: 1
Trailing spaces
419-419: Expected: 0 or 2; Actual: 1
Trailing spaces
502-502: Expected: 0 or 2; Actual: 1
Trailing spaces
504-504: Expected: 0 or 2; Actual: 1
Trailing spaces
512-512: Expected: 0 or 2; Actual: 1
Trailing spaces
538-538: Expected: 0 or 2; Actual: 1
Trailing spaces
541-541: Expected: 0 or 2; Actual: 1
Trailing spaces
597-597: Expected: 0 or 2; Actual: 1
Trailing spaces
599-599: Expected: 0 or 2; Actual: 1
Trailing spaces
628-628: Expected: 0 or 2; Actual: 1
Trailing spaces
649-649: Expected: 0 or 2; Actual: 1
Trailing spaces
691-691: Expected: 0 or 2; Actual: 1
Trailing spaces
11-11: Expected: 1; Actual: 2
Multiple consecutive blank lines
469-469: Expected: 1; Actual: 2
Multiple consecutive blank lines
640-640: Expected: 1; Actual: 2
Multiple consecutive blank lines
691-691: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
700-700: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
716-716: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
719-719: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
723-723: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
726-726: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
727-727: Expected: 1; Actual: 0; Above
Headings should be surrounded by blank lines
692-692: null
Lists should be surrounded by blank lines
701-701: null
Lists should be surrounded by blank lines
717-717: null
Lists should be surrounded by blank lines
720-720: null
Lists should be surrounded by blank lines
724-724: null
Lists should be surrounded by blank lines
98-98: null
Bare URL used
40-40: null
Fenced code blocks should have a language specified
Additional comments not posted (10)
peers/pane.go (1)
100-101: The added TODO comment is clear and effectively references a GitHub issue for tracking the resolution of the potential crash scenario.sock.go (3)
17-17: The new imports and types are appropriate for the added functionality regarding ICE candidate handling.Also applies to: 45-57
211-258: The modifications to thehandleStatusfunction are well-implemented and effectively support the new feature of displaying ICE candidate information.
454-458: The newWritemethod in theCandidatePairValuestype is correctly implemented and usestabwriter.Writereffectively to format the ICE candidate data.webexec.go (6)
21-21: Added import fortext/tabwriter.This import is necessary for the new functionality in the
statusCMDfunction to format output.
25-25: Added import forgithub.com/fatih/color.This import is used to add color-coded output in the
statusCMDfunction, enhancing the user interface.
394-394: Usage ofnewSocketClientin theacceptfunction.The refactored
newSocketClientis correctly used here to establish a connection.
Line range hint
489-507: RefactoredgetAgentCandidatesto use the newhttpcparameter.The change aligns with the new design to pass the HTTP client around, which is a good practice for dependency injection.
507-507: Changes ingetCandidateto also use the newhttpcparameter.Consistent with other changes to ensure that all network operations use the passed-in HTTP client.
538-607: Enhancements instatusCMDto include detailed status output with color coding and tabular data.The enhancements improve the usability of the status command, making the output more readable and informative.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.modis excluded by!**/*.mod
Files selected for processing (4)
- CHANGELOG.md (1 hunks)
- peers/peer.go (3 hunks)
- sock.go (2 hunks)
- webexec.go (9 hunks)
Files skipped from review as they are similar to previous changes (1)
- sock.go
Additional context used
LanguageTool
CHANGELOG.md
[grammar] ~7-~7: Consider using either the past participle “defined” or the present participle “defining” here. (BEEN_PART_AGREEMENT)
Context: ...pec/v2.0.0.html). his file's format is define in [Keep a Changelog](https://keepacha...
[grammar] ~104-~104: The word ‘install’ is not a noun. (A_INSTALL)
Context: ... ## [1.0.1] 2023-8-3 ### Fixed - The install script ## [1.0.0] 2023-7-31 ### Fixed...
[grammar] ~182-~182: The word ‘install’ is not a noun. (A_INSTALL)
Context: ....9] 2022-10-06 ### Fixed - updted the install script which was stuck on 0.17.6 ### [...
[grammar] ~213-~213: Did you mean Apple’s computer “Mac” (= trademark, capitalized)? (APPLE_PRODUCTS)
Context: ...staller messages are much improved on a mac ## [0.17.4] 2022-08-13 ### Fixed - r...
[grammar] ~245-~245: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ... - Handling of PeerBook disconnection - MacOS one line installer ## [0.16.0] 2022-01...
[duplication] ~366-~366: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...ed to add their key ### Fixed - crash on on closing an already closed peerbook conn...
[uncategorized] ~403-~403: Do not mix variants of the same word (‘signalling’ and ‘signaling’) within a single text. (EN_EXACT_COHERENCY_RULE)
Context: ...xed - passing candidates regardless of signalling state (which is unreliable) ## [0.10.1...
[grammar] ~537-~537: There seems to be a noun/verb agreement error. Did you mean “errs” or “erred”? (SINGULAR_NOUN_VERB_AGREEMENT)
Context: ....8.1] - 2020-2-4 ### Added - redirect err to an error file - Added pion logging c...
[uncategorized] ~611-~611: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’). (AUTO_HYPHEN)
Context: ...ow ## [0.5.4] - 2020-1-5 ### Added - Auto releasing with wide os & architecture support ##...
[uncategorized] ~652-~652: This expression is usually spelled with a hyphen. (BASED_HYPHEN)
Context: ... to store output - Orderly shutdown and marker based restore ## [0.4.3] - 2020-11-24 ### F...
[uncategorized] ~683-~683: This expression is usually spelled with a hyphen. (BASED_HYPHEN)
Context: ... ## [0.4.0] - 2020-10-14 ### Fixed - Linter based code beutification ### Added - Screen...
[grammar] ~692-~692: This phrase is duplicated. You should probably use “sub commands” only once. (PHRASE_REPETITION)
Context: ...t runs in the backgroung and managed by sub commands - Sub commands - help, auth, start, stop, status, rest...
[uncategorized] ~717-~717: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...## Changed - Authentication is based on secret which can be either a password or a has...
Markdownlint
CHANGELOG.md
3-3: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
7-7: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
68-68: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
70-70: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
102-102: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
130-130: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
139-139: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
174-174: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
224-224: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
230-230: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
236-236: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
240-240: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
280-280: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
282-282: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
318-318: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
338-338: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
342-342: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
345-345: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
373-373: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
379-379: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
388-388: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
391-391: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
413-413: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
419-419: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
502-502: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
504-504: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
512-512: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
538-538: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
541-541: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
597-597: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
599-599: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
628-628: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
649-649: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
691-691: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
11-11: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
469-469: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
640-640: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
691-691: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
700-700: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
716-716: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
719-719: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
723-723: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
726-726: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
727-727: Expected: 1; Actual: 0; Above (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
692-692: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
701-701: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
717-717: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
720-720: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
724-724: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
98-98: null (MD034, no-bare-urls)
Bare URL used
40-40: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
golangci-lint
peers/peer.go
55-55: undefined: CTRLMessage (typecheck)
37-37: undefined: NewClientsDB (typecheck)
279-279: undefined: Pane (typecheck)
345-345: undefined: Pane (typecheck)
368-368: undefined: CTRLMessage (typecheck)
384-384: undefined: CTRLMessage (typecheck)
283-283: undefined: Pane (typecheck)
306-306: undefined: ParseWinsize (typecheck)
325-325: undefined: NewPane (typecheck)
346-346: undefined: Panes (typecheck)
369-369: undefined: AckArgs (typecheck)
385-385: undefined: NAckArgs (typecheck)
404-404: undefined: AckArgs (typecheck)
410-410: undefined: NAckArgs (typecheck)
620-620: undefined: Panes (typecheck)
Additional comments not posted (11)
CHANGELOG.md (1)
16-16: The changelog entry is clear and aligns well with the PR objectives.peers/peer.go (3)
14-14: The addition of thetext/tabwriterimport is appropriate for the new functionality to format tabular data.
80-96: TheCandidatePairStatsstruct and itsWritemethod are well-implemented for handling ICE candidate pair statistics.
499-538: TheGetCandidatePairmethod is logically correct but references several undefined types such asPane,Panes, andCTRLMessage. Please ensure these are defined or imported correctly.+ import "path/to/required/package" // Adjust the import path as necessaryLikely invalid or redundant comment.
webexec.go (7)
21-21: Added importtext/tabwriterto support tabular output formatting.
25-25: Added importgithub.com/fatih/colorto support color-coded output.
350-364: RefactorednewSocketClientto handle Unix socket connections.
427-427: TheTrickleCandidatesfunction now accepts anhttp.Clientas a parameter, aligning with the changes in other related functions to facilitate better HTTP client management.
489-489: ThegetAgentCandidatesfunction now accepts anhttp.Clientas a parameter, which is consistent with the changes inTrickleCandidatesand other related functions.
507-507: ThegetCandidatefunction signature has been modified to accept anhttp.Clientas a parameter. This change is part of a broader refactoring to improve HTTP client management across the application.
538-606: Enhanced thestatusCMDfunction to include color-coded output and detailed status information about connected peers, including ICE candidates. This change aligns with the PR's objective to enhance thestatuscommand.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores