edvin / tornadofx

Lightweight JavaFX Framework for Kotlin
Apache License 2.0
3.67k stars 269 forks source link

Integrate CrashFX #51

Closed mikehearn closed 7 years ago

mikehearn commented 8 years ago

This is more of a reminder to myself than a request, but last year I wrote a crash handler library for JavaFX:

https://github.com/vinumeris/crashfx

It came with a simple web collector/dashboarding thing, although it's very crude and not suitable for high volumes of crash reports.

It doesn't make sense to maintain this as a separate library. I think it'd be better to merge the code into TornadoFX to upgrade the built in crash dialog.

What do you think?

thomasnield commented 8 years ago

I'm not quite sure about Hibernate dependencies and all that. We would prefer to keep TornadoFX simple and with as few dependencies as possible.

I haven't tried out your library yet but I see nothing wrong with dialogs and lightweight logging if it is an improvement. I'll defer to Edvin.

edvin commented 8 years ago

Hi Mike,

This is very interesting, and kind of a coincidence - i was just thinking about a better way to handle exceptions, including finding a way to report back to the developer.

It looks like you've done all the hard work already :) This could absolutely fit inside TornadoFX, if we can find a "pluggable" way to report the exceptions, maybe even with a couple of default implementations like email, http upload (for example to the CrashFX server etc). I haven't looked at CrashFX in detail yet, so I don't know how you actually transfer the exceptions etc.

@thomasnield - I don't think Mike is proposing to add any dependencies on the Web part of CrashFX into TornadoFX.

What do you think, Mike - could you propose a good way to integrate it?

thomasnield commented 8 years ago

Sorry I misunderstood. Sounds great! I'm going to check it out too.

mikehearn commented 8 years ago

Yes, sure, I'm super busy right now with something else but I'll probably get some free time to make myself useful in a few weeks.

Right - no dependency on the web part. They're two separate projects that just happen to be in one git repo.

Reporting via email is tricky. Direct SMTP submission isn't much better than just direct HTTP submission, especially as a lot of networks block SMTP outbound to control spam. Opening up an email app is quite easy if you don't care about setting the body, but we do. Finally, you do want some way to usefully aggregate the reports, and that in turns suggests the reports need to go to a database.

So for these reasons CrashFX only supports submission via HTTP[S].

What it actually does is write a crash report to disk and then the crash is reported on the next run, when you initialise the library. That's because the user doesn't want to be delayed in restarting the app, most likely, even if they have a slow connection. It also allows for natural background retries in case the collector has problems, or the users internet is unavailable.

There'd also be benefit in stealing a bit of code from UpdateFX, which is my own update library a bit like yours Edvin, except with a different design. UpdateFX has a function to restart the process by finding the executable in the right place on each platform and running it. It only works if you use javapackager, but is mightily convenient for the apps that do.

edvin commented 8 years ago

Great to hear! No rush, and let me know if I can be of assistance. You are right about SMTP, it might be useful for in-house apps with dedicated access to an SMTP-server, but there really is no reason to support anything but an HTTP-interface. The HTTP-backend could forward emails if that's what the user wants.

We could probably reuse the http payload you use in CrashFX, and then users can choose to use CrashFX on the server side, or just implement the same server side response if they want to handle the reports differently. Does that make sense? (Haven't had the time to look at the server side of it yet).

If I understand you correctly, you see apps that crash in an unrecoverable manner, and then CrashFX collects and subsequently report this error on the next run, right? My experience is that most errors have to do with remoting or the occasional ui bug, which is often recoverable and doesn't require a restart of the app.

I like the idea of saving the crash report locally in case the network is down/slow (that might even be the cause of the error) and then uploading later, but it should be possible to report and continue working, depending on the nature of the error.

Cool, I like the UpdateFX functionality, would be nice to include that as well. Even if it just works for javapackager it's a big win. I'm going to take a look at wrapping FXLauncher in a Launch4J binary when I have the time - i suspect that would work in a similar fashion to javapackager :)

Btw, your presentation on Functional Programming with Kotlin was one of the reasons I decided to switch over to Kotlin, and by extension create TornadoFX :)

mikehearn commented 8 years ago

Well, that's great to hear :)

Yes, I'd like to extend CrashFX so you can choose to ignore and continue after an exception. That's a pretty basic feature.

The server report is basically just a line based text format that can include recent log lines (CrashFX includes a rolling log buffer so it can submit the last N lines along with the crash).

edvin commented 7 years ago

As discussed on Slack, we will close this issue for now and add features from CrashFX as they are needed.