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

Online crash reporting. See #1853 #2313

Merged
merged 5 commits into from Mar 15, 2017

Conversation

Projects
None yet
2 participants
@lazka
Member

lazka commented Mar 11, 2017

Most of the error reporting is now in quodlibet.errorreport

  • Logging the error and printing to stdout
  • Writing the error + recent logs to disk
  • Showing the error to the user
  • Opening a pre-filled github issue
  • Sending the error sentry.io (github fallback)
  • Logging/reporting segfault stacktraces

Depends on raven, which is unlikely to be installed atm, maybe we'll vendor it.

@lazka

This comment has been minimized.

Member

lazka commented Mar 13, 2017

This now includes raven and contextlib2 directly in git and will also install it. They both don't get included in the source tarball and the code falls back to prefilled gihub issues in those cases. Couldn't think of anything better..

raven required quite a bit of monkey patching to get working for our use case, but since we only use the internal version this should be fine I guess.

I consider this mergeable, feedback welcome.

@declension

This comment has been minimized.

Member

declension commented Mar 13, 2017

Good work! All seems to function properly here, though I haven't got the Sentry login to check anything on the other side.

The bit that scares me slightly is the diff: +14,804 −490 lines.
Is there a way of pulling raven (in a build step maybe), and monkey patching the original one - or is that too tricky (haven't seen the code really)?

Alternatively is an egg / wheel in Git an option - then there's a cleaner distinction between vendor code and QL code (and devs / IDEs don't see that code so much)? I guess it would still need an explicit build step which is a bit weird for Python (maybe...).

@lazka

This comment has been minimized.

Member

lazka commented Mar 14, 2017

Good work! All seems to function properly here, though I haven't got the Sentry login to check anything on the other side.

Thanks. I'll look into making it possible for more people to see the data.

https://sentry.io/share/issue/3134323431352e323334303630333835/

The bit that scares me slightly is the diff: +14,804 −490 lines.
Is there a way of pulling raven (in a build step maybe), and monkey patching the original one - or is that too tricky (haven't seen the code really)?

Build step would work somehow. The upside now is that we can test it on travis.

Alternatively is an egg / wheel in Git an option - then there's a cleaner distinction between vendor code and QL code (and devs / IDEs don't see that code so much)? I guess it would still need an explicit build step which is a bit weird for Python (maybe...).

I haven't checked, but zip import might work. Is there any problem with your IDE and the current solution?

See bebd0ef for the code needed to mark quodlibet.optpackages as "not part of the project".

All in all I'm open to rework this in the future if an alternative arises.

@lazka

This comment has been minimized.

Member

lazka commented Mar 14, 2017

Thanks. I'll look into making it possible for more people to see the data.

I've asked if we can get another user.

@declension

This comment has been minimized.

Member

declension commented Mar 14, 2017

I haven't checked, but zip import might work. Is there any problem with your IDE and the current solution?

Zip import sounds good. No problems as such, more just being in the source tree it gets treated like real source code, e.g. notifying of code problems (>1000 warnings there). I suppose we can just configure IDEs to also ignore that folder tree.

BTW do we need any of optpackages/raven/contrib (support for Django / Celery / Tornado etc)?

All in all I'm open to rework this in the future if an alternative arises.

OK so maybe let's get it in now and think of something to do with build steps / zip imports separately...

@lazka lazka force-pushed the lazka:errorreport-sentry branch from 7b95648 to 231afe6 Mar 14, 2017

lazka added some commits Mar 11, 2017

Online crash reporting. See #1853
Most of the error reporting is now in quodlibet.errorreport

* Logging the error and printing to stdout
* Writing the error + recent logs to disk
* Showing the error to the user
* Opening a pre-filled github issue
* Sending the error sentry.io (github fallback)
* Logging/reporting segfault stacktraces

Depends on raven, which is unlikely to be installed atm, maybe we'll vendor it.
Add support for including raven/contextlib2 in git
The packages get integrated through an import hook which only allows
the local version to get loaded. The content will net get shipped
in the tarball and is excluded from the various source code tests.
optpackages: Include raven/contextlib2
Except raven.contrib which we don't need and
the certs file because we monkey patch raven to use our own.
optpackages: add script for generating the content
Since we now delete some files from the pip downloaded source
this makes it easier to reproduce the result with newer versions.

@lazka lazka force-pushed the lazka:errorreport-sentry branch from 231afe6 to 1ae4de9 Mar 14, 2017

@lazka

This comment has been minimized.

Member

lazka commented Mar 14, 2017

BTW do we need any of optpackages/raven/contrib (support for Django / Celery / Tornado etc)?

Thanks. I've removed that now and automated the process: 0c026ea

(and sqashed/reorderd the patches..)

@lazka lazka merged commit ec75da5 into quodlibet:master Mar 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lazka

This comment has been minimized.

Member

lazka commented Mar 17, 2017

Thanks. I'll look into making it possible for more people to see the data.

I've asked if we can get another user.

Our account got upgraded now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment