No uncaught service exceptions are logged out of the box.

[trustin]

When a new user writes an Armeria server, the user often doesn't realize they have to add LoggingService to see the actual exception in the log. This default behavior can be very confusing. It would be nice if we can log the uncaught service exceptions by default somehow without worrying too much about double notification.

One idea:

• Add a new writable property RequestContext.shouldLogUncaughtException which is true by default.
• Modify LoggingClient and LoggingService to set false to shouldLogUncaughtException.

Thoughts?

[ikhoon]

Overall it is a useful feature. That being said, as an error of a client request is propagated via the returned HttpResponse, I prefer to add shouldLogUncaughtException only to ServiceRequestContext.

[alexc-db]

This uncaught exception log can be setup as a periodic task that logs summary of the uncaught exceptions, something like

[LOG_LEVEL] Observed X uncaught exceptions in last PERIOD seconds. If you don't see error logs - please use LoggingService decorator.

LOG_LEVEL and PERIOD can be configured via server builder, use e.g. ERROR and 60 by default, respectively. This way users can opt out by changing log level or setting PERIOD to e.g. -1.

Or, if we'd like to generalize this even more - we could have a log message that returns multiple stats like that

[INFO] Server stats:
...
- Unhandled errors: X (LoggingService disabled)
...

In my experience periodic summaries like that are super useful for debugging because they can store detailed internal state summary for the service, easy to find, provides insight even when other metrics (e.g. Prometheus) are missing.

[trustin]

If we log the counter message, we could uncount the uncaught exceptions whose ctx.shouldLogUncaughtException is false to make the counter log message disappears when all ervices are covered by LoggingService or other handlers that call ctx.setShouldLogUncaughtException(false).

Given logging every uncaught exceptions can be very spammy, we could:

• Log the counter message as suggested by @alexc-db; and
• Maybe log the most recently uncaught exception as an example when logging the counter? (This should be useful when a user is testing their server.)

[seonWKim]

Hi, I've few questions while trying to solve this issue.

• If we have to add shouldLogUncaughtException to the ServiceRequestContext(which is a public interface), we have to add some getter and setter methods for shouldLogUncaughtException. Will it be okay to add methods on this public interface?
  • ex) boolean shouldLogUncaughtException() or void setShouldLogUncaughtException(boolean value)
  • There's only 2 implementations of ServiceRequestContext in armeria project so it's not a lot of work to add new methods to implementations.
• Should we report uncaught exceptions periodically by using some sort of daemon threads?
  • Or how about logging uncaught exceptions information when a new exception occurs and some specific time has passed after last logged time? This will be less noisy and log only when some exceptions occur.

[trustin]

• Yes, you can add the public methods.
• You don't need a dedicated thread for doing this. You can get one of the boss or worker event loop from ServerConfig when Server starts itself and schedule a periodic task using scheduleWithFixedDelay(). You need to keep the ScheduledFuture returned by scheduleWithFixedDelay() so you can cancel it when Server stops itself.