Crash reporting shouldn't be static

Signed-off-by: David Gageot <david@gageot.net>

commands/commands.go

@@ -103,8 +103,6 @@ func fatalOnError(command func(commandLine CommandLine, api libmachine.API) erro
 		api.GithubAPIToken = context.GlobalString("github-api-token")
 		api.Filestore.Path = context.GlobalString("storage-path")
 
-		crashreport.Configure(context.GlobalString("bugsnag-api-token"))
-
 		// TODO (nathanleclaire): These should ultimately be accessed
 		// through the libmachine client by the rest of the code and
 		// not through their respective modules.  For now, however,
@@ -116,6 +114,11 @@ func fatalOnError(command func(commandLine CommandLine, api libmachine.API) erro
 
 		if err := command(&contextCommandLine{context}, api); err != nil {
 			log.Fatal(err)
+
+			if crashErr, ok := err.(crashreport.CrashError); ok {
+				crashReporter := crashreport.NewCrashReporter(mcndirs.GetBaseDir(), context.GlobalString("bugsnag-api-token"))
+				crashReporter.Send(crashErr)
+			}
 		}
 	}
 }

libmachine/crashreport/crash_report.go

@@ -16,7 +16,6 @@ import (
 	"io/ioutil"
 
 	"github.com/bugsnag/bugsnag-go"
-	"github.com/docker/machine/commands/mcndirs"
 	"github.com/docker/machine/libmachine/log"
 	"github.com/docker/machine/libmachine/mcnutils"
 	"github.com/docker/machine/version"
@@ -27,28 +26,53 @@ const (
 	noreportAPIKey = "no-report"
 )
 
-var apiKey string
+type CrashReporter interface {
+	Send(err CrashError) error
+}
 
-// Configure the apikey for bugnag
-func Configure(key string) {
+// CrashError describes an error that should be reported to bugsnag
+type CrashError struct {
+	Cause       error
+	Command     string
+	Context     string
+	DriverName  string
+	LogFilePath string
+}
+
+func (e CrashError) Error() string {
+	return e.Cause.Error()
+}
+
+type BugsnagCrashReporter struct {
+	baseDir string
+	apiKey  string
+}
+
+// NewCrashReporter creates a new bugsnag based CrashReporter. Needs an apiKey.
+func NewCrashReporter(baseDir string, apiKey string) *BugsnagCrashReporter {
+	if apiKey == "" {
 		apiKey = defaultAPIKey
-	if key != "" {
-		apiKey = key
+	}
+
+	return &BugsnagCrashReporter{
+		baseDir: baseDir,
+		apiKey:  apiKey,
 	}
 }
 
-func SendWithFile(err error, context string, driverName string, command string, path string) error {
-	if noReportFileExist() || apiKey == noreportAPIKey {
+// Send sends a crash report to bugsnag via an http call.
+func (r *BugsnagCrashReporter) Send(err CrashError) error {
+	if r.noReportFileExist() || r.apiKey == noreportAPIKey {
 		log.Debug("Opting out of crash reporting.")
 		return nil
 	}
 
-	if apiKey == "" {
+	if r.apiKey == "" {
 		return errors.New("Not sending report since no api key has been set.")
 	}
 
 	bugsnag.Configure(bugsnag.Configuration{
-		APIKey: apiKey,
+		APIKey: r.apiKey,
 		// XXX we need to abuse bugsnag metrics to get the OS/ARCH information as a usable filter
 		// Can do that with either "stage" or "hostname"
 		ReleaseStage:    fmt.Sprintf("%s (%s)", runtime.GOOS, runtime.GOARCH),
@@ -68,19 +92,23 @@ func SendWithFile(err error, context string, driverName string, command string,
 	detectRunningShell(&metaData)
 	detectUname(&metaData)
 	detectOSVersion(&metaData)
-	addFile(path, &metaData)
+	addFile(err.LogFilePath, &metaData)
 
 	var buffer bytes.Buffer
 	for _, message := range log.History() {
 		buffer.WriteString(message + "\n")
 	}
 	metaData.Add("history", "trace", buffer.String())
-	return bugsnag.Notify(err, metaData, bugsnag.SeverityError, bugsnag.Context{String: context}, bugsnag.ErrorClass{Name: fmt.Sprintf("%s/%s", driverName, command)})
+
+	return bugsnag.Notify(err.Cause, metaData, bugsnag.SeverityError, bugsnag.Context{String: err.Context}, bugsnag.ErrorClass{Name: fmt.Sprintf("%s/%s", err.DriverName, err.Command)})
 }
 
-// Send through http the crash report to bugsnag need a call to Configure(apiKey) before
-func Send(err error, context string, driverName string, command string) error {
-	return SendWithFile(err, context, driverName, command, "")
+func (r *BugsnagCrashReporter) noReportFileExist() bool {
+	optOutFilePath := filepath.Join(r.baseDir, "no-error-report")
+	if _, err := os.Stat(optOutFilePath); os.IsNotExist(err) {
+		return false
+	}
+	return true
 }
 
 func addFile(path string, metaData *bugsnag.MetaData) {
@@ -97,14 +125,6 @@ func addFile(path string, metaData *bugsnag.MetaData) {
 	metaData.Add("logfile", filepath.Base(path), string(data))
 }
 
-func noReportFileExist() bool {
-	optOutFilePath := filepath.Join(mcndirs.GetBaseDir(), "no-error-report")
-	if _, err := os.Stat(optOutFilePath); os.IsNotExist(err) {
-		return false
-	}
-	return true
-}
-
 func detectRunningShell(metaData *bugsnag.MetaData) {
 	shell := os.Getenv("SHELL")
 	if shell != "" {

libmachine/host/host.go

@@ -146,14 +146,22 @@ func (h *Host) Upgrade() error {
 
 	provisioner, err := provision.DetectProvisioner(h.Driver)
 	if err != nil {
-		crashreport.Send(err, "provision.DetectProvisioner", h.Driver.DriverName(), "Upgrade")
-		return err
+		return crashreport.CrashError{
+			Cause:      err,
+			Command:    "Upgrade",
+			Context:    "provision.DetectProvisioner",
+			DriverName: h.Driver.DriverName(),
+		}
 	}
 
 	log.Info("Upgrading docker...")
 	if err := provisioner.Package("docker", pkgaction.Upgrade); err != nil {
-		crashreport.Send(err, "provisioner.Package", h.Driver.DriverName(), "Upgrade")
-		return err
+		return crashreport.CrashError{
+			Cause:      err,
+			Command:    "Upgrade",
+			Context:    "provisioner.Package",
+			DriverName: h.Driver.DriverName(),
+		}
 	}
 
 	log.Info("Restarting docker...")

libmachine/libmachine.go

@@ -132,8 +132,18 @@ func (api *Client) Create(h *host.Host) error {
 	log.Info("Creating machine...")
 
 	if err := api.performCreate(h); err != nil {
-		sendCrashReport(err, api, h)
-		return err
+		vBoxLog := ""
+		if h.DriverName == "virtualbox" {
+			vBoxLog = filepath.Join(api.GetMachinesDir(), h.Name, h.Name, "Logs", "VBox.log")
+		}
+
+		return crashreport.CrashError{
+			Cause:       err,
+			Command:     "Create",
+			Context:     "api.performCreate",
+			DriverName:  h.DriverName,
+			LogFilePath: vBoxLog,
+		}
 	}
 
 	log.Debug("Reticulating splines...")
@@ -185,15 +195,6 @@ func (api *Client) performCreate(h *host.Host) error {
 	return nil
 }
 
-func sendCrashReport(err error, api *Client, host *host.Host) {
-	if host.DriverName == "virtualbox" {
-		vboxlogPath := filepath.Join(api.GetMachinesDir(), host.Name, host.Name, "Logs", "VBox.log")
-		crashreport.SendWithFile(err, "api.performCreate", host.DriverName, "Create", vboxlogPath)
-	} else {
-		crashreport.Send(err, "api.performCreate", host.DriverName, "Create")
-	}
-}
-
 func (api *Client) Close() error {
 	return api.clientDriverFactory.Close()
 }