server.Start: recover and log panics

[bmoylan]

This change is 

[nmiyake]

Reviewed 2 of 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @nmiyake and @bmoylan)

---

witchcraft/witchcraft.go, line 394 at r1 (raw file):

				rErr = err
			} else {
				rErr = werror.Error("panic recovered", werror.UnsafeParam("recovered", r))

Does this double-wrap? (the svc1log.Error call uses "panic recovered", so do we get "panic recovered" twice in this scenario?)

Also, we should update function documentation to state that all panics are recovered and logged and that, if any panic is encountered, the returned error will be non-nil and be the information on the panic (and only information on the panic -- any other error that was being returned will be overwritten)

[bmoylan]

---

witchcraft/witchcraft.go, line 394 at r1 (raw file):

Previously, nmiyake (Nick Miyake) wrote…

Does this double-wrap? (the svc1log.Error call uses "panic recovered", so do we get "panic recovered" twice in this scenario?)

Also, we should update function documentation to state that all panics are recovered and logged and that, if any panic is encountered, the returned error will be non-nil and be the information on the panic (and only information on the panic -- any other error that was being returned will be overwritten)

This one (the error message) will end up in the stacktrace field, while the one below will end up in the message field.

[nmiyake]

Thanks,

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

[nmiyake]

(except for the build failing issue :)