Tweaks to Notices

* Disable "ignored server" notice due to noise in diagnostics
* Report critical ConsoleClient errors as notices (e.g., datastore init errors)
* Add special notice for upstream proxy error messages that may need to
  be relayed to users

AndroidLibrary/psi/psi.go

@@ -57,16 +57,18 @@ func Start(configJson, embeddedServerEntryList string, provider PsiphonProvider)
 	config.DeviceBinder = provider
 	config.DnsServerGetter = provider
 
-	err = psiphon.InitDataStore(config)
-	if err != nil {
-		return fmt.Errorf("error initializing datastore: %s", err)
-	}
-
 	psiphon.SetNoticeOutput(psiphon.NewNoticeReceiver(
 		func(notice []byte) {
 			provider.Notice(string(notice))
 		}))
 
+	// TODO: should following errors be Notices?
+
+	err = psiphon.InitDataStore(config)
+	if err != nil {
+		return fmt.Errorf("error initializing datastore: %s", err)
+	}
+
 	serverEntries, err := psiphon.DecodeAndValidateServerEntryList(embeddedServerEntryList)
 	if err != nil {
 		log.Fatalf("error decoding embedded server entry list: %s", err)

ConsoleClient/psiphonClient.go

@@ -23,7 +23,6 @@ import (
 	"flag"
 	"io"
 	"io/ioutil"
-	"log"
 	"os"
 	"os/signal"
 	"runtime/pprof"
@@ -50,42 +49,56 @@ func main() {
 
 	flag.Parse()
 
+	// Initialize default Notice output (stderr)
+
+	var noticeWriter io.Writer
+	noticeWriter = os.Stderr
+	if formatNotices {
+		noticeWriter = psiphon.NewNoticeConsoleRewriter(noticeWriter)
+	}
+	psiphon.SetNoticeOutput(noticeWriter)
+
 	// Handle required config file parameter
 
 	if configFilename == "" {
-		log.Fatalf("configuration file is required")
+		psiphon.NoticeError("configuration file is required")
+		os.Exit(1)
 	}
 	configFileContents, err := ioutil.ReadFile(configFilename)
 	if err != nil {
-		log.Fatalf("error loading configuration file: %s", err)
+		psiphon.NoticeError("error loading configuration file: %s", err)
+		os.Exit(1)
 	}
 	config, err := psiphon.LoadConfig(configFileContents)
 	if err != nil {
-		log.Fatalf("error processing configuration file: %s", err)
+		psiphon.NoticeError("error processing configuration file: %s", err)
+		os.Exit(1)
 	}
 
-	// Initialize notice output; use logfile, if configured
+	// When a logfile is configured, reinitialize Notice output
 
-	var noticeWriter io.Writer
-	noticeWriter = os.Stderr
 	if config.LogFilename != "" {
 		logFile, err := os.OpenFile(config.LogFilename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600)
 		if err != nil {
-			log.Fatalf("error opening log file: %s", err)
+			psiphon.NoticeError("error opening log file: %s", err)
+			os.Exit(1)
 		}
 		defer logFile.Close()
+		var noticeWriter io.Writer
+		noticeWriter = logFile
+		if formatNotices {
+			noticeWriter = psiphon.NewNoticeConsoleRewriter(noticeWriter)
+		}
+		psiphon.SetNoticeOutput(noticeWriter)
 	}
-	if formatNotices {
-		noticeWriter = psiphon.NewNoticeConsoleRewriter(noticeWriter)
-	}
-	psiphon.SetNoticeOutput(noticeWriter)
 
 	// Handle optional profiling parameter
 
 	if profileFilename != "" {
 		profileFile, err := os.Create(profileFilename)
 		if err != nil {
-			log.Fatalf("error opening profile file: %s", err)
+			psiphon.NoticeError("error opening profile file: %s", err)
+			os.Exit(1)
 		}
 		pprof.StartCPUProfile(profileFile)
 		defer pprof.StopCPUProfile()
@@ -95,7 +108,8 @@ func main() {
 
 	err = psiphon.InitDataStore(config)
 	if err != nil {
-		log.Fatalf("error initializing datastore: %s", err)
+		psiphon.NoticeError("error initializing datastore: %s", err)
+		os.Exit(1)
 	}
 
 	// Handle optional embedded server list file parameter
@@ -105,18 +119,21 @@ func main() {
 	if embeddedServerEntryListFilename != "" {
 		serverEntryList, err := ioutil.ReadFile(embeddedServerEntryListFilename)
 		if err != nil {
-			log.Fatalf("error loading embedded server entry list file: %s", err)
+			psiphon.NoticeError("error loading embedded server entry list file: %s", err)
+			os.Exit(1)
 		}
 		// TODO: stream embedded server list data? also, the cast makaes an unnecessary copy of a large buffer?
 		serverEntries, err := psiphon.DecodeAndValidateServerEntryList(string(serverEntryList))
 		if err != nil {
-			log.Fatalf("error decoding embedded server entry list file: %s", err)
+			psiphon.NoticeError("error decoding embedded server entry list file: %s", err)
+			os.Exit(1)
 		}
 		// Since embedded server list entries may become stale, they will not
 		// overwrite existing stored entries for the same server.
 		err = psiphon.StoreServerEntries(serverEntries, false)
 		if err != nil {
-			log.Fatalf("error storing embedded server entry list data: %s", err)
+			psiphon.NoticeError("error storing embedded server entry list data: %s", err)
+			os.Exit(1)
 		}
 	}
 
@@ -124,7 +141,8 @@ func main() {
 
 	controller, err := psiphon.NewController(config)
 	if err != nil {
-		log.Fatalf("error creating controller: %s", err)
+		psiphon.NoticeError("error creating controller: %s", err)
+		os.Exit(1)
 	}
 
 	controllerStopSignal := make(chan struct{}, 1)

psiphon/TCPConn_unix.go

@@ -73,6 +73,14 @@ func interruptibleTCPDial(addr string, config *DialConfig) (conn *TCPConn, err e
 	dialAddr := addr
 	if config.UpstreamHttpProxyAddress != "" {
 		dialAddr = config.UpstreamHttpProxyAddress
+
+		// Report connection errors in a notice, as user may have input
+		// invalid proxy address or credential
+		defer func() {
+			if err != nil {
+				NoticeUpstreamProxyError(err)
+			}
+		}()
 	}
 
 	// Get the remote IP and port, resolving a domain name if necessary
@@ -169,7 +177,7 @@ func interruptibleTCPDial(addr string, config *DialConfig) (conn *TCPConn, err e
 	// Going through upstream HTTP proxy
 	if config.UpstreamHttpProxyAddress != "" {
 		// This call can be interrupted by closing the pending conn
-		err := HttpProxyConnect(conn, addr)
+		err = HttpProxyConnect(conn, addr)
 		if err != nil {
 			return nil, ContextError(err)
 		}

psiphon/TCPConn_windows.go

@@ -75,6 +75,9 @@ func interruptibleTCPDial(addr string, config *DialConfig) (conn *TCPConn, err e
 
 		if err == nil && config.UpstreamHttpProxyAddress != "" {
 			err = HttpProxyConnect(netConn, addr)
+			if err != nil {
+				NoticeUpstreamProxyError(err)
+			}
 		}
 		if err != nil {
 			netConn = nil

psiphon/config.go

@@ -29,7 +29,7 @@ import (
 // TODO: allow all params to be configured
 
 const (
-	VERSION                                      = "0.0.8"
+	VERSION                                      = "0.0.9"
 	DATA_STORE_FILENAME                          = "psiphon.db"
 	CONNECTION_WORKER_POOL_SIZE                  = 10
 	TUNNEL_POOL_SIZE                             = 1

psiphon/dataStore.go

@@ -186,7 +186,10 @@ func StoreServerEntry(serverEntry *ServerEntry, replaceIfExists bool) error {
 			return ContextError(err)
 		}
 		if serverEntryExists && !replaceIfExists {
-			NoticeInfo("ignored update for server %s", serverEntry.IpAddress)
+			// Disabling this notice, for now, as it generates too much noise
+			// in diagnostics with clients that always submit embedded servers
+			// to the core on each run.
+			// NoticeInfo("ignored update for server %s", serverEntry.IpAddress)
 			return nil
 		}
 		_, err = transaction.Exec(`

psiphon/notice.go

@@ -174,6 +174,12 @@ func NoticeSplitTunnelRegion(region string) {
 	outputNotice("SplitTunnelRegion", true, "region", region)
 }
 
+// NoticeUpstreamProxyError reports an error when connecting to an upstream proxy. The
+// user may have input, for example, an incorrect address or incorrect credentials.
+func NoticeUpstreamProxyError(err error) {
+	outputNotice("UpstreamProxyError", true, "errorMessage", fmt.Sprintf("%s", err))
+}
+
 type noticeObject struct {
 	NoticeType string          `json:"noticeType"`
 	Data       json.RawMessage `json:"data"`