From aa1cbca4c415f00edee5583f658a23ec036ad610 Mon Sep 17 00:00:00 2001
From: James Casey <james@chef.io>
Date: Thu, 6 Dec 2018 04:46:07 -0800
Subject: [PATCH] Remove panic by moving problem code out of constructor

The root of the panic is due to us trying to serialize a
`Toml::Value::Table` to a Vec<u8> inside a constructor where we can't
handle the error cleanly

This commit pulls the TOML parsing out of Service::new() into
Cfg::to_exported() so we can properly handle the Result<> on toml parsing

Note: this does not yet handle the underlying issue which is that the
TOML serializer doesn't handle tables well.  In particular it requires
all keys with non-table values must be emitted first.  This is not the
default

See https://github.com/alexcrichton/toml-rs/blob/master/src/ser.rs#L7-L12

Signed-off-by: James Casey <james@chef.io>
---
 components/butterfly/src/rumor/service.rs    | 8 ++------
 components/sup/src/manager/service/config.rs | 4 ++--
 components/sup/src/manager/service/mod.rs    | 2 +-
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/components/butterfly/src/rumor/service.rs b/components/butterfly/src/rumor/service.rs
index 73c3275dc5c..f44fc91b134 100644
--- a/components/butterfly/src/rumor/service.rs
+++ b/components/butterfly/src/rumor/service.rs
@@ -87,7 +87,7 @@ impl Service {
         package: &T,
         service_group: ServiceGroup,
         sys: SysInfo,
-        cfg: Option<&toml::value::Table>,
+        cfg: Option<Vec<u8>>,
     ) -> Self
     where
         T: Identifiable,
@@ -110,11 +110,7 @@ impl Service {
             initialized: false,
             pkg: package.to_string(),
             sys: sys,
-            // TODO FN: Can we really expect this all the time, should we return a `Result<Self>`
-            // in this constructor?
-            cfg: cfg
-                .map(|v| toml::ser::to_vec(v).expect("Struct should serialize to bytes"))
-                .unwrap_or_default(),
+            cfg: cfg.unwrap_or_default(),
         }
     }
 }
diff --git a/components/sup/src/manager/service/config.rs b/components/sup/src/manager/service/config.rs
index 92b90e8b137..ae8b18d6bf8 100644
--- a/components/sup/src/manager/service/config.rs
+++ b/components/sup/src/manager/service/config.rs
@@ -227,7 +227,7 @@ impl Cfg {
     }
 
     /// Returns a subset of the overall configuration which intersects with the given package's exports.
-    pub fn to_exported(&self, pkg: &Pkg) -> Result<toml::value::Table> {
+    pub fn to_exported(&self, pkg: &Pkg) -> Result<Vec<u8>> {
         let mut map = toml::value::Table::default();
         let cfg = toml::Value::try_from(&self).unwrap();
         for (key, path) in pkg.exports.iter() {
@@ -253,7 +253,7 @@ impl Cfg {
                 map.insert(key.clone(), curr.clone());
             }
         }
-        Ok(map)
+        toml::ser::to_vec(&map).map_err(|e| sup_error!(Error::TomlEncode(e)))
     }
 
     fn load_toml_file<T1, T2>(dir: T1, file: T2) -> Result<Option<toml::value::Table>>
diff --git a/components/sup/src/manager/service/mod.rs b/components/sup/src/manager/service/mod.rs
index 332555c47a1..828b733218b 100644
--- a/components/sup/src/manager/service/mod.rs
+++ b/components/sup/src/manager/service/mod.rs
@@ -681,7 +681,7 @@ impl Service {
             &self.pkg.ident,
             self.service_group.clone(),
             self.sys.as_sys_info().clone(),
-            exported.as_ref(),
+            exported,
         );
         rumor.incarnation = incarnation;
         rumor