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

Updated Crash reporter #848

Merged
merged 17 commits into from May 4, 2012
Merged

Updated Crash reporter #848

merged 17 commits into from May 4, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 26, 2012

This crash reporter reports crashes based on:

  • Plugins that didn't load
  • If the OS created a new Quicksilver.crash file in the ~/Library/Logs/CrashReporter

It's much improved over the last one and shouldn't give any false positives. It also has the following features:

  • Sends crash reports to the QSApp server
  • Allows for user comments to try and reproduce the problem
  • Clears the QS caches automatically
  • Provides access to the FAQ and Issue tracker should the user with to see them
  • Looks Sexy
  • Improves load time of plugin (as less writing is required to the QuicksilverState.plist file)

I suggest that reviewers of the code don't look through the commits of the QSCrashReporterWindowController files, since they're completely new. Just see the end products.

To simulate a crash, alter the lastKnownCrashDate in the com.blacktree.Quicksivler.plist prefs file to be some time in the past. E.g. to be 21 Apr 2008 00:28:24

To simulate a plugin crash, alter ~/Library/Caches/Quicksilver/QuicksilverState.plist to include the name of a plugin and a path such as (for the Cyberduck Module):

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>QSFaultyPluginPath</key>
    <string>/Users/name/Library/Application Support/Quicksilver/PlugIns/com.blacktree.Quicksilver.Cyberduck.36.qsplugin</string>
    <key>QSFaultyPluginName</key>
    <string>Cyberduck Module</string>
</dict>
</plist>

The server side code used to receive the file is as follows (please check there are no security loop holes in this)

<?php
$file = "reports/".$_POST['name'];
$fh = fopen($file, 'w') or die("can't open file");
$userComments = "User Comments: ".$_POST['comments']."\n\n\n------------------ CRASH REPORT -----------------\n\n\n";
$stringData = $_POST['data'];
fwrite($fh, $userComments.$stringData);
fclose($fh);
?>

pjrobertson added 8 commits Apr 26, 2012
* Adds a crash reporter .xib window
* Adds HTML files for displaying custom text depending on the crash type (plugin load or overall crash)
* Detects a crash by checking the files in ~/Library/Logs/CrashReporter/Quicksilver-xxxx.crash
* Reduces dependency on ~/Library/Caches/Quicksilver/QuicksilverState.plist, improving start up time
Ensures that if the user deletes the plugin, the info.dict can still be obtained for sending to the server
Also - minor adjustments to the crash reporter window
@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 26, 2012

Some pictures:
http://d.pr/i/NtOy for typical crashes
http://d.pr/i/QU51 for plugin crashes

Also - add the deletePlugin method to the header file
@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 26, 2012

I'd love to see some more comments/documentation.
And I think the server side code does need some work

But I'll look it over more thoroughly tomorrow.

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Apr 27, 2012

I have added comments and improved the robustness of the PHP script. QS now also sends a useful User-Agent string to the server

To view the changes to the script, please either see the file at qs0/crashreports/reporter.php on the server (if you have access) or direct message me on IRC: irc.freenote.net#quicksilver (user pjrobertson)
I am not showing it here for security reasons.

@skurfer
Copy link
Member

@skurfer skurfer commented May 2, 2012

I seem to be getting a false positive here. If you install a new plug-in, the next time you launch QS, it claims that plug-in caused a crash.

Also, maybe “Delete Plugin” should be a checkbox, and the delete will take place when you click one of the Send/Don’t Send buttons. It seems weird to me that I have to click two buttons (or close the panel manually). But I can see why you don’t want the button to dismiss the window. What if the user wants to delete the plug-in and send a report?

pjrobertson added 3 commits May 3, 2012
Also, release the Window controller once the window is closed
* Make the `deletePlugin` option a checkbox
* Add a progress indicator
* Don't delete a plugin if the user explicitly closes the window
* Fix a memory leak - release the crashReportPath when the window closes
- (void)deletePlugin;
- (IBAction)openCrashReportsWikiPage:(id)sender;

@end
Copy link
Member

@skurfer skurfer May 3, 2012

Choose a reason for hiding this comment

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

If (only if) we find something else that needs to be changed, maybe add a newline here?

pjrobertson added 2 commits May 3, 2012
There's no need, since the plugin will only be deleted just before the window is closed
Also plugIns not plugins. Stupid!
@skurfer
Copy link
Member

@skurfer skurfer commented May 4, 2012

OK, I’m happy.

skurfer added a commit that referenced this issue May 4, 2012
@skurfer skurfer merged commit 1766b87 into quicksilver:master May 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants