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

Exceptions are stored on disk even if offline mode is off and are never sent #63

Closed
bragma opened this issue Jan 13, 2016 · 3 comments
Closed
Assignees
Labels
Milestone

Comments

@bragma
Copy link

bragma commented Jan 13, 2016

I've noticed this odd (probably wrong) behavior associated to the AutoCollection of Exceptions. In a brief: if "enableOfflineMode" if off, unhandled exceptions are not sent and pile up on disk.

Details follow:
If AutoCollection is enabled, all uncaught exceptions are trapped by "uncaughtException" event on the process. This causes the exception data to be sent to "handleCrash" method on Channel object with "isNodeCrashing" set to true. "handleCrash" sends the exception data to "saveOnCrash" and this method does not check if "_enableOfflineMode" is true. It just stores on disk.

If "_enableOfflineMode" if false, 2 (bad) things happen: Files are stored on disk even if this functionality is off. It may still make sense, since it's the only way to make unhandled exception collection work. But since "_enableOfflineMode" is false, files stored on disk are never sent.

So, ultimately:

  1. Files will pile up forever on disk
  2. Unhandled Exceptions will not be sent

Maybe it would be better to:

  1. Not store exceptions to disk, since they will not be sent. It may happen that on first run offline mode is off, then switched on later. But its an odd scenario. If this is not the case, Exception files will be piled up forever.
  2. Try to send exceptions (only) on next start even if offline mode is off.

What do you think?

@bragma bragma changed the title Exceptions are stored on disk even if option is disabled, and the never sent Exceptions are stored on disk even if offline mode is off and are never sent Jan 13, 2016
@southwood southwood added the bug label Jan 21, 2016
@southwood
Copy link
Contributor

Hi @bragma, thanks for writing this up. From what you're saying and looking at the code it looks like you're assessment is correct and this was probably a regression during one of the offlineMode changes. It should be noted that the crash handling is has been impacted by several of the last version changes of node so it's also possible that this is a version-specific issue.

I like your proposal. Separating crashes from the 'offlineMode' concept seems like the right thing to do. Feel free to make a pull request or ask @BogdanBe to triage.

@bragma
Copy link
Author

bragma commented Mar 14, 2016

@southwood sadly I'm really busy right now. I'd happily pick up the PR but I think this problem deserves more care I can afford right now. :(

@jsheely
Copy link

jsheely commented Apr 7, 2016

This should address this issue. #83

Though I believe more thought will need to be put into how offlineMode works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants