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

TCP Dump Bug Fix #1291

Merged
merged 1 commit into from
Mar 4, 2025
Merged

TCP Dump Bug Fix #1291

merged 1 commit into from
Mar 4, 2025

Conversation

tejhan
Copy link
Collaborator

@tejhan tejhan commented Mar 4, 2025

Slight bug observed where if a user escapes the file selection menu after clicking the button to download a TCP dump capture, the button stays stuck in the disabled downloading state.
image
This PR adds an error response to correct for that.

(To reproduce, select Troubleshoot Network Health > Collect TCP Dumps, select a node, start & stop a capture, then click the download button that appears next to completed capture item)

.vsix: vscode-aks-tools-1.6.1-tcpfix.vsix.zip

@tejhan tejhan added the bug Something isn't working label Mar 4, 2025
@tejhan tejhan requested a review from Copilot March 4, 2025 13:05
@tejhan tejhan self-assigned this Mar 4, 2025
@tejhan tejhan requested a review from Tatsinnit March 4, 2025 13:05

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR fixes a bug where the TCP dump download button remains disabled if a user cancels the file selection.

  • Adds a check for missing file selection.
  • Sends an error response to reset the download state.

Reviewed Changes

File Description
src/panels/TcpDumpPanel.ts Added error handling to reset the download state when no file is chosen.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/panels/TcpDumpPanel.ts:422

  • [nitpick] Ensure consistency in error messaging style across the application. If similar error responses elsewhere follow a different format, consider aligning this message accordingly.
//  Reset download state with error message if no file path was selected
Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡Thank you so much for this! That's really nice, thank you so much, I will test this soon. Code looks good.

Copy link
Collaborator

@ReinierCC ReinierCC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Tested on Windows, worked well.

@Tatsinnit Tatsinnit merged commit 67b6c1b into Azure:main Mar 4, 2025
9 checks passed
@Tatsinnit Tatsinnit mentioned this pull request Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants