Repository: tipsy/javalin (kotlin)

Issue:
Add a lifecycle event for server stop failed
**Describe the feature**
Add a lifecycle event for server stop failed.

**Additional context**
Based on the [documentation](https://javalin.io/documentation#lifecycle-events), the `serverSt*` lifecycle events are presumably designed to mirror [Jetty's lifecycle events](https://github.com/eclipse/jetty.project/blob/jetty-10.0.x/jetty-util/src/main/java/org/eclipse/jetty/util/component/LifeCycle.java). Either way, the difference is that Javalin wisely separated failed into start failed. However, stop can fail too. It makes sense to have both events. Is it intentional to not have a stop failed event?

Furthermore, it would be nice to obtain the exception that caused the `serverStartFailed` and `serverStopFailed` events. What do you think about adding a new `ExceptionalEventHandler` that the failure events use? A downside is that this will become baked into the API and in the future a failed start or failed stop may not be initiated by an exception. This might also require a breaking change.

I'm happy to provide a PR if you think this is a good idea.

Thanks.

Pull request:
Hi Tipsy. 

Resolves #1442. Tried to maintain your code style. If merged, the [docs](https://javalin.io/documentation#lifecycle-events) need updating. Let me know your thoughts. Note: this changes the contract for `stop()` because it now throws, as it should have originally.

Thanks.

Patch:
diff --git a/javalin/src/main/java/io/javalin/Javalin.java b/javalin/src/main/java/io/javalin/Javalin.java
index 6dc01ad72..ffdb3c079 100644
--- a/javalin/src/main/java/io/javalin/Javalin.java
+++ b/javalin/src/main/java/io/javalin/Javalin.java
@@ -209,7 +209,9 @@ public Javalin stop() {
         try {
             jettyServer.server().stop();
         } catch (Exception e) {
+            eventManager.fireEvent(JavalinEvent.SERVER_STOP_FAILED);
             JavalinLogger.error("Javalin failed to stop gracefully", e);
+            throw new JavalinException(e);
         }
         JavalinLogger.info("Javalin has stopped");
         eventManager.fireEvent(JavalinEvent.SERVER_STOPPED);
diff --git a/javalin/src/main/java/io/javalin/core/event/EventListener.java b/javalin/src/main/java/io/javalin/core/event/EventListener.java
index b216ae793..528358a94 100644
--- a/javalin/src/main/java/io/javalin/core/event/EventListener.java
+++ b/javalin/src/main/java/io/javalin/core/event/EventListener.java
@@ -21,6 +21,7 @@ public EventListener(EventManager eventManager) {
     public void serverStarting(@NotNull EventHandler eventHandler)    {  addLifecycleEvent(JavalinEvent.SERVER_STARTING,     eventHandler); }
     public void serverStarted(@NotNull EventHandler eventHandler)     {  addLifecycleEvent(JavalinEvent.SERVER_STARTED,      eventHandler); }
     public void serverStartFailed(@NotNull EventHandler eventHandler) {  addLifecycleEvent(JavalinEvent.SERVER_START_FAILED, eventHandler); }
+    public void serverStopFailed(@NotNull EventHandler eventHandler)  {  addLifecycleEvent(JavalinEvent.SERVER_STOP_FAILED, eventHandler); }
     public void serverStopping(@NotNull EventHandler eventHandler)    {  addLifecycleEvent(JavalinEvent.SERVER_STOPPING,     eventHandler); }
     public void serverStopped(@NotNull EventHandler eventHandler)     {  addLifecycleEvent(JavalinEvent.SERVER_STOPPED,      eventHandler); }
     // @formatter:on
diff --git a/javalin/src/main/java/io/javalin/core/event/EventManager.kt b/javalin/src/main/java/io/javalin/core/event/EventManager.kt
index ac577329e..759fe5b3f 100644
--- a/javalin/src/main/java/io/javalin/core/event/EventManager.kt
+++ b/javalin/src/main/java/io/javalin/core/event/EventManager.kt
@@ -20,6 +20,6 @@ class EventManager {
     fun fireWsHandlerAddedEvent(metaInfo: WsHandlerMetaInfo) = wsHandlerAddedHandlers.apply { this.forEach { it.accept(metaInfo) } }
 }
 
-enum class JavalinEvent { SERVER_STARTING, SERVER_STARTED, SERVER_START_FAILED, SERVER_STOPPING, SERVER_STOPPED }
+enum class JavalinEvent { SERVER_STARTING, SERVER_STARTED, SERVER_START_FAILED, SERVER_STOP_FAILED, SERVER_STOPPING, SERVER_STOPPED }
 data class HandlerMetaInfo(val httpMethod: HandlerType, val path: String, val handler: Any, val roles: Set<RouteRole>)
 data class WsHandlerMetaInfo(val handlerType: WsHandlerType, val path: String, val handler: Any, val roles: Set<RouteRole>)


Test patch:
diff --git a/javalin/src/test/java/io/javalin/TestLifecycleEvents.kt b/javalin/src/test/java/io/javalin/TestLifecycleEvents.kt
index a1a9de7e5..34f373cae 100644
--- a/javalin/src/test/java/io/javalin/TestLifecycleEvents.kt
+++ b/javalin/src/test/java/io/javalin/TestLifecycleEvents.kt
@@ -7,9 +7,14 @@
 
 package io.javalin
 
+import io.javalin.core.util.JavalinException
+import io.javalin.jetty.JettyServer
 import io.javalin.testing.TestUtil
+import io.mockk.every
+import io.mockk.mockk
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 
 class TestLifecycleEvents {
 
@@ -27,6 +32,40 @@ class TestLifecycleEvents {
         assertThat(log).isEqualTo("StartingStartedStoppingStoppingStoppingStopped")
     }
 
+    @Test
+    fun `serverStartFailed event works`() =
+            `test failed lifecycle event`("StartingStartFailed") {
+                every { start(any()) } throws RuntimeException("expected")
+            }
+
+    @Test
+    fun `serverStopFailed event works`() =
+            `test failed lifecycle event`("StartingStartedStoppingStopFailed") {
+                every { server().stop() } throws RuntimeException("expected")
+            }
+
+
+    private fun `test failed lifecycle event`(expected: String, mockBlock: JettyServer.() -> Unit) {
+        val jettyServer = mockk<JettyServer>(relaxed = true)
+        mockBlock(jettyServer)
+        val app = Javalin(jettyServer, null)
+        var log = ""
+        app.events {
+            it.serverStartFailed { log += "StartFailed" }
+            it.serverStopFailed { log += "StopFailed" }
+            it.serverStarting { log += "Starting" }
+            it.serverStarted { log += "Started" }
+            it.serverStopping { log += "Stopping" }
+            it.serverStopped { log += "Stopped" }
+        }
+
+        val exception = assertThrows<JavalinException> { app.start(0).stop() }
+
+        assertThat(log).isEqualTo(expected)
+        assertThat(exception.cause).isInstanceOf(RuntimeException::class.java)
+        assertThat(exception.cause!!.message).isEqualTo("expected")
+    }
+
     @Test
     fun `handlerAdded event works`() = TestUtil.test { app, _ ->
         var log = ""