From 959d45c5b58db05c589cd98d93f609ec66696d0e Mon Sep 17 00:00:00 2001
From: Dan Bond <danbond@protonmail.com>
Date: Sat, 11 Jun 2022 17:11:01 -0700
Subject: [PATCH] serve: don't panic if server already running

Signed-off-by: Dan Bond <danbond@protonmail.com>
---
 Cargo.toml      |  2 +-
 src/server.rs   | 19 +++++++++----------
 tests/server.rs |  4 ++--
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 9436971..9082bce 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "metrics_server"
-version = "0.8.0"
+version = "0.8.1"
 authors = ["Dan Bond <danbond@protonmail.com>"]
 edition = "2021"
 rust-version = "1.58"
diff --git a/src/server.rs b/src/server.rs
index 49037d0..9deb4ac 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -23,7 +23,7 @@ impl MetricsServer {
     ///
     /// # Panics
     ///
-    /// Panics if given an invalid address.
+    /// Panics if given an invalid address or incorrect TLS credentials.
     pub fn new<A>(addr: A, certificate: Option<Vec<u8>>, private_key: Option<Vec<u8>>) -> Self
     where
         A: ToSocketAddrs,
@@ -98,13 +98,12 @@ impl MetricsServer {
     /// Start serving requests on the underlying server.
     ///
     /// The server will only respond synchronously as it blocks until receiving new requests.
-    ///
-    /// # Panics
-    ///
-    /// Panics if called on a server that has already been started.
     pub fn serve(mut self) -> Self {
-        if self.thread.is_some() {
-            panic!("metrics_server already started")
+        // Check if we already have a thread running.
+        if let Some(thread) = &self.thread {
+            if !thread.is_finished() {
+                return self;
+            }
         }
 
         // Invoking clone on Arc produces a new Arc instance, which points to the
@@ -161,14 +160,14 @@ impl MetricsServer {
 }
 
 impl Drop for MetricsServer {
-    // TODO: should I really be doing this inside drop? It _could_ panic,
-    // so maybe a shutdown method would be better?
+    // TODO: should we really be doing this inside drop? It _could_ panic,
+    // so maybe a shutdown/stop method would be better?
     fn drop(&mut self) {
         // Signal that we should stop handling requests and unblock the server.
         self.shared.stop.store(true, Ordering::Relaxed);
         self.shared.server.unblock();
 
-        // Because join takes ownership of the thread, we need call the take method
+        // Because join takes ownership of the thread, we need to call the take method
         // on the Option to move the value out of the Some variant and leave a None
         // variant in its place.
         if let Some(thread) = self.thread.take() {
diff --git a/tests/server.rs b/tests/server.rs
index 91eccd7..6bc907d 100644
--- a/tests/server.rs
+++ b/tests/server.rs
@@ -34,11 +34,11 @@ fn test_new_server_invalid_key() {
 }
 
 #[test]
-#[should_panic]
 fn test_new_server_already_running() {
     let srv = MetricsServer::new("localhost:8002", None, None).serve();
 
-    // Attempt to start an already running server.
+    // Attempt to start an already running server should be ok
+    // as we will return the pre-existing thread.
     srv.serve();
 }