Intentionally panic when unable to write logs

- This is desired when: proceeding without
  logging is unacceptable; psiphond is run
  as a managed service that will be restarted,
  which may resolve the issue; panics are
  captured and monitored.
- Can be disabled with SkipPanickingLogWriter
  config parameter.

psiphon/remoteServerList_test.go

@@ -76,6 +76,9 @@ func TestObfuscatedRemoteServerLists(t *testing.T) {
 			TunnelProtocolPorts:  map[string]int{"OSSH": 4001},
 			LogFilename:          filepath.Join(testDataDirName, "psiphond.log"),
 			LogLevel:             "debug",
+
+			// "defer os.RemoveAll" will cause a log write error
+			SkipPanickingLogWriter: true,
 		})
 	if err != nil {
 		t.Fatalf("error generating server config: %s", err)

psiphon/server/api.go

@@ -98,8 +98,12 @@ func dispatchAPIRequestHandler(
 	// terminating in the case of a bug.
 	defer func() {
 		if e := recover(); e != nil {
-			log.LogPanicRecover(e, debug.Stack())
-			reterr = common.ContextError(errors.New("request handler panic"))
+			if intentionalPanic, ok := e.(IntentionalPanicError); ok {
+				panic(intentionalPanic.AddStack(debug.Stack()))
+			} else {
+				log.LogPanicRecover(e, debug.Stack())
+				reterr = common.ContextError(errors.New("request handler panic"))
+			}
 		}
 	}()
 

psiphon/server/config.go

@@ -63,10 +63,9 @@ type Config struct {
 	// to. When blank, logs are written to stderr.
 	LogFilename string
 
-	// PanicLogFilename specifies the path of the file to
-	// log unrecovered panics to. When blank, logs are
-	// written to stderr
-	PanicLogFilename string
+	// SkipPanickingLogWriter disables panicking when
+	// unable to write any logs.
+	SkipPanickingLogWriter bool
 
 	// DiscoveryValueHMACKey is the network-wide secret value
 	// used to determine a unique discovery strategy.
@@ -343,14 +342,14 @@ func validateNetworkAddress(address string, requireIPaddress bool) error {
 // GenerateConfigParams specifies customizations to be applied to
 // a generated server config.
 type GenerateConfigParams struct {
-	LogFilename          string
-	PanicLogFilename     string
-	LogLevel             string
-	ServerIPAddress      string
-	WebServerPort        int
-	EnableSSHAPIRequests bool
-	TunnelProtocolPorts  map[string]int
-	TrafficRulesFilename string
+	LogFilename            string
+	SkipPanickingLogWriter bool
+	LogLevel               string
+	ServerIPAddress        string
+	WebServerPort          int
+	EnableSSHAPIRequests   bool
+	TunnelProtocolPorts    map[string]int
+	TrafficRulesFilename   string
 }
 
 // GenerateConfig creates a new Psiphon server config. It returns JSON
@@ -501,7 +500,7 @@ func GenerateConfig(params *GenerateConfigParams) ([]byte, []byte, []byte, error
 	config := &Config{
 		LogLevel:                       logLevel,
 		LogFilename:                    params.LogFilename,
-		PanicLogFilename:               params.PanicLogFilename,
+		SkipPanickingLogWriter:         params.SkipPanickingLogWriter,
 		GeoIPDatabaseFilenames:         nil,
 		HostID:                         "example-host-id",
 		ServerIPAddress:                params.ServerIPAddress,

psiphon/server/log.go

@@ -203,6 +203,33 @@ func InitLogging(config *Config) (retErr error) {
 				retErr = common.ContextError(err)
 				return
 			}
+
+			if !config.SkipPanickingLogWriter {
+
+				// Use PanickingLogWriter, which will intentionally
+				// panic when a Write fails. Set SkipPanickingLogWriter
+				// if this behavior is not desired.
+				//
+				// Note that NewRotatableFileWriter will first attempt
+				// a retry when a Write fails.
+				//
+				// It is assumed that continuing operation while unable
+				// to log is unacceptable; and that the psiphond service
+				// is managed and will restart when it terminates.
+				//
+				// It is further assumed that panicking will result in
+				// an error that is externally logged and reported to a
+				// monitoring system.
+				//
+				// TODO: An orderly shutdown may be preferred, as some
+				// data will be lost in a panic (e.g., server_tunnel logs).
+				// It may be possible to perform an orderly shutdown first
+				// and then panic, or perform an orderly shutdown and
+				// simulate a panic message that will be reported.
+
+				logWriter = NewPanickingLogWriter(config.LogFilename, logWriter)
+			}
+
 		} else {
 			logWriter = os.Stderr
 		}

psiphon/server/utils.go

@@ -26,6 +26,8 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
+	"fmt"
+	"io"
 	"math/big"
 	"time"
 
@@ -126,3 +128,60 @@ func GenerateWebServerCertificate(commonName string) (string, string, error) {
 
 	return string(webServerCertificate), string(webServerPrivateKey), nil
 }
+
+// IntentionalPanicError is an error type that is used
+// when calling panic() in a situation where recovers
+// should propagate the panic.
+type IntentionalPanicError struct {
+	message string
+}
+
+// NewIntentionalPanicError creates a new IntentionalPanicError.
+func NewIntentionalPanicError(message string) error {
+	return IntentionalPanicError{message: message}
+}
+
+// Error implements the error interface.
+func (err IntentionalPanicError) Error() string {
+	return err.message
+}
+
+// AddStack creates a new IntentionalPanicError which
+// records the given stack. When a IntentionalPanicError is
+// recovered, call AddStack with the debug.Stack() at the
+// point of recovery, and panic with the resulting
+// IntentionalPanicError.
+func (err IntentionalPanicError) AddStack(debugStack []byte) string {
+	return NewIntentionalPanicError(
+		fmt.Sprintf("intentional panic error: %s\nstack: %s\n",
+			intentionalPanic.Error(),
+			string(debugStack)))
+}
+
+// PanickingLogWriter wraps an io.Writer and intentionally
+// panics when a Write() fails.
+type PanickingLogWriter struct {
+	name   string
+	writer io.Writer
+}
+
+// NewPanickingLogWriter creates a new PanickingLogWriter.
+func NewPanickingLogWriter(
+	name string, writer io.Writer) *PanickingLogWriter {
+
+	return &PanickingLogWriter{
+		name:   name,
+		writer: writer,
+	}
+}
+
+// Write implements the io.Writer interface.
+func (w *PanickingLogWriter) Write(p []byte) (n int, err error) {
+	n, err = w.writer.Write(p)
+	if err != nil {
+		panic(
+			NewIntentionalPanicError(
+				fmt.Sprintf("fatal write to %s failed: %s", w.name, err)))
+	}
+	return
+}