vlingo / xoom-actors

The VLINGO XOOM platform SDK for the type-safe Actor Model, delivering Reactive concurrency, high scalability, high-throughput, and resiliency using Java and other JVM languages.
https://vlingo.io
Mozilla Public License 2.0
229 stars 28 forks source link

Exceptions should never be suppressed #45

Closed alanStrait closed 5 years ago

alanStrait commented 5 years ago

In using vlingo for application development the default actor supervisor policy seems to cause exceptions to be lost without being logged. While the documentation is clear that actor lifecycle methods should be overridden to understand why an actor has been stopped, it would be valuable for the default policy to, “log and stop” rather than just “stop.”

VaughnVernon commented 5 years ago

@alanStrait Thanks, this is good. Do you mean like this?

  protected void beforeRestart(final Throwable reason) {
    // override
    logger().log("Default recovery after: " + reason.getMessage(), reason);
    lifeCycle.afterStop(this);
  }
alanStrait commented 5 years ago

Yes, that would be helpful, it not a full stack trace (when in development mode).

VaughnVernon commented 5 years ago

@alanStrait Ok, will do. I think that all of the default supervisors log, but I may be wrong. If you can identify any supervisors missing logging I can fix those, too.

VaughnVernon commented 5 years ago

@alanStrait The Actor default recovery methods now implementing logging. Please test and close if you agree that it's fixed. (Again if you find other places that are missing essential logging please create and issue.)

alanStrait commented 5 years ago

Yes, that would be helpful, if not a full stack trace (for development mode).

Alan Strait | m.+1.720.840.8567

On Apr 26, 2019, at 12:52 PM, Vaughn Vernon notifications@github.com wrote:

@alanStrait https://github.com/alanStrait Thanks, this is good. Do you mean like this?

protected void beforeRestart(final Throwable reason) { // override logger().log("Default recovery after: " + reason.getMessage(), reason); lifeCycle.afterStop(this); } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vlingo/vlingo-actors/issues/45#issuecomment-487164003, or mute the thread https://github.com/notifications/unsubscribe-auth/ABJ2FYN5NI6QNHRZRTRQL23PSNFOPANCNFSM4HIZQTFA.

VaughnVernon commented 5 years ago

@alanStrait The full stacktrace should log/print with the standard logging. If you agree, please Close this issue. See:

https://github.com/vlingo/vlingo-actors/blob/e6926cdf0a12978b40a7c4ffa103eb720732f599/src/main/java/io/vlingo/actors/plugin/logging/jdk/JDKLogger.java#L72

alanStrait commented 5 years ago

After picking up the change I reverted some source code to expose an NPE that I would have hoped to be logged. Unfortunately the behavior remains as before, no indication of what is wrong but also does not complete restful call. I'd suggest closing this and perhaps I'll be able to collect more information that may be helpful as time goes on.

VaughnVernon commented 5 years ago

Implemented. There may be other issues with recovery that are app/service specific. Closing.