Show an error message when pcap slices can't be opened #3114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jameskerr
reviewed
Jul 2, 2024
Contributor
jameskerr
left a comment
jameskerr
left a comment
There was a problem hiding this comment.
Makes good sense to me.
jameskerr
approved these changes
Jul 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tl;dr
Per #1379, we've had problems on some platforms when a user clicks Download Packets and there's no application installed to open the extracted slice, e.g., Wireshark. Though we've merged some changes over time that have helped us improve in this area, this change covers us for Linux where the problem is still significant.
Fixes #1379
Details
As of the most recent update in #1379 (comment), Zui actually gets decent help here from the OS on both macOS and Windows since in those cases the OS pops up a notification about the lack of a helper app for pcaps. However, on Linux the user would still see just the Packets extracted. Opening... message from Zui, no launched pcap app, and no other guidance, so there was still room for improvement there.
I pursued an angle advocated in #1379 (comment) of researching what's returned by the function call to open the pcap helper app, which is currently Electron's
shell.openPath(). What I found is that it does sometimes return an error message in the absence of a helper app, though it's not the most helpful. Specifically:Because these messages are pretty vague, in the branch for this PR I'm instead opting to display the more explicit error message Could not open extracted pcap. Is Wireshark installed? Granted, in theory a user could install a different pcap helper app instead of Wireshark, but it seems like 99% of our users think of Wireshark here, and if a user is advanced enough to be seeking alternate helper app I imagine they'll get the essence of what the message is implying and install the app of their choosing.
The fact there's no message returned at all on Windows is annoying, but since we have the OS pop-up to cover us there, I'm not inclined to get obsessed.
Tests
Using a Dev build generated from this branch, here's video on each OS showing evidence of the failure case (no Wireshark installed) and success case (just to confirm we've not disturbed what was already working ok).
Linux
Failure
This is the main benefit we're getting here, since the user now sees our error message instead of the silence from before.
Linux-fail.mp4
Success
Linux-success.mp4
Windows
Failure
Because
shell.openPath()returns no error message here, we rely on the OS to inform the user.Windows-fail.mp4
Success
Windows-success.mp4
macOS
Failure
Here we get the "belt & suspenders" benefit of both the error message from Zui and the pop-up from the OS.
macOS-fail.mp4
Success
macOS-success.mp4