Fix Microsoft Windows crashes #313
Merged
Conversation
This file contains 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
tombruijn
force-pushed
the
windows
branch
2 times, most recently
from
5937dc9
to
b2206f4
Compare
This is not to fix Microsoft Windows support entirely, but just to allow users to install AppSignal without errors on the OS. This does not guarantee AppSignal working in its entirety on Microsoft's OS. This change uses a different method to find the tmp dir for Windows while keeping the original logic for *NIX and macOS. Because the logic is a bit more dynamic now for the system's tmp dir I moved it to a method that's stubbed for most specs, except the ones that test the actual implementation of the method.
When the extension is not loaded running the diagnose task will error out when parsing the agent report when requested from the extension. This change checks beforehand if the extension is loaded or not.
Print done notice even when we don't detect the framework. Then also send a demo sample. Hopefully the red "Warning" will attract people's attention that even thought the installation has been completed we still can't detect the framework. Printing the done notice also prints the warning when a user is installing AppSignal on Windows.
Don't save the username on Microsoft Windows, but also don't error the program. The `Etc.getpwuid(uid)` returns `nil` on Windows.
Write output immediately to STDOUT on Microsoft Windows. Without `$stdout.sync = true` the output would be saved until the end of the program and then flushed to STDOUT. When we print the output immediately we don't get the illusion of a hanging installer and diagnose task when we ask for user input. Because the user input prompt hasn't been flushed to STDOUT the user doesn't see what's expected of them to do. This change fixes that.
`getch` throws a `Errno::EBADF: Bad file descriptor` error on Microsoft Windows. I am unable to fix that. Using `getc` creates the same behavior and also works on Microsoft Windows. This fixes the "Press any key" prompts on Microsoft's OS, fixing the installer prompts.
matsimitsu
approved these changes
Jun 18, 2017
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.
Fixes #311
Steps to test
appsignal install PUSH_API_KEY(with an without a supported framework as a dependency to the app)appsignal diagnoseTODO