X Tutup
Skip to content

Commit ac7e323

Browse files
dgageotdgageot
committed
Merge pull request docker-archive-public#2775 from dgageot/fix-bugsnag
Fix Bugnag Report not being sent
2 parents 7387519 + f96595d commit ac7e323

File tree

4 files changed

+87
-23
lines changed

4 files changed

+87
-23
lines changed

‎commands/commands.go‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ var (
2121
ErrNoMachineSpecified = errors.New("Error: Expected to get one or more machine names as arguments")
2222
ErrExpectedOneMachine = errors.New("Error: Expected one machine name as an argument")
2323
ErrTooManyArguments = errors.New("Error: Too many arguments given")
24+
25+
osExit = func(code int) { os.Exit(code) }
2426
)
2527

2628
// CommandLine contains all the information passed to the commands on the command line.
@@ -92,7 +94,7 @@ func runAction(actionName string, c CommandLine, api libmachine.API) error {
9294
return nil
9395
}
9496

95-
func fatalOnError(command func(commandLine CommandLine, api libmachine.API) error) func(context *cli.Context) {
97+
func runCommand(command func(commandLine CommandLine, api libmachine.API) error) func(context *cli.Context) {
9698
return func(context *cli.Context) {
9799
api := libmachine.NewClient(mcndirs.GetBaseDir(), mcndirs.GetMachineCertDir())
98100
defer api.Close()
@@ -113,12 +115,14 @@ func fatalOnError(command func(commandLine CommandLine, api libmachine.API) erro
113115
ssh.SetDefaultClient(api.SSHClientType)
114116

115117
if err := command(&contextCommandLine{context}, api); err != nil {
116-
log.Fatal(err)
118+
log.Error(err)
117119

118120
if crashErr, ok := err.(crashreport.CrashError); ok {
119121
crashReporter := crashreport.NewCrashReporter(mcndirs.GetBaseDir(), context.GlobalString("bugsnag-api-token"))
120122
crashReporter.Send(crashErr)
121123
}
124+
125+
osExit(1)
122126
}
123127
}
124128
}
@@ -140,13 +144,13 @@ var Commands = []cli.Command{
140144
{
141145
Name: "active",
142146
Usage: "Print which machine is active",
143-
Action: fatalOnError(cmdActive),
147+
Action: runCommand(cmdActive),
144148
},
145149
{
146150
Name: "config",
147151
Usage: "Print the connection config for machine",
148152
Description: "Argument is a machine name.",
149-
Action: fatalOnError(cmdConfig),
153+
Action: runCommand(cmdConfig),
150154
Flags: []cli.Flag{
151155
cli.BoolFlag{
152156
Name: "swarm",
@@ -159,14 +163,14 @@ var Commands = []cli.Command{
159163
Name: "create",
160164
Usage: "Create a machine",
161165
Description: fmt.Sprintf("Run '%s create --driver name' to include the create flags for that driver in the help text.", os.Args[0]),
162-
Action: fatalOnError(cmdCreateOuter),
166+
Action: runCommand(cmdCreateOuter),
163167
SkipFlagParsing: true,
164168
},
165169
{
166170
Name: "env",
167171
Usage: "Display the commands to set up the environment for the Docker client",
168172
Description: "Argument is a machine name.",
169-
Action: fatalOnError(cmdEnv),
173+
Action: runCommand(cmdEnv),
170174
Flags: []cli.Flag{
171175
cli.BoolFlag{
172176
Name: "swarm",
@@ -190,7 +194,7 @@ var Commands = []cli.Command{
190194
Name: "inspect",
191195
Usage: "Inspect information about a machine",
192196
Description: "Argument is a machine name.",
193-
Action: fatalOnError(cmdInspect),
197+
Action: runCommand(cmdInspect),
194198
Flags: []cli.Flag{
195199
cli.StringFlag{
196200
Name: "format, f",
@@ -203,18 +207,18 @@ var Commands = []cli.Command{
203207
Name: "ip",
204208
Usage: "Get the IP address of a machine",
205209
Description: "Argument(s) are one or more machine names.",
206-
Action: fatalOnError(cmdIP),
210+
Action: runCommand(cmdIP),
207211
},
208212
{
209213
Name: "kill",
210214
Usage: "Kill a machine",
211215
Description: "Argument(s) are one or more machine names.",
212-
Action: fatalOnError(cmdKill),
216+
Action: runCommand(cmdKill),
213217
},
214218
{
215219
Name: "ls",
216220
Usage: "List machines",
217-
Action: fatalOnError(cmdLs),
221+
Action: runCommand(cmdLs),
218222
Flags: []cli.Flag{
219223
cli.BoolFlag{
220224
Name: "quiet, q",
@@ -240,7 +244,7 @@ var Commands = []cli.Command{
240244
Name: "regenerate-certs",
241245
Usage: "Regenerate TLS Certificates for a machine",
242246
Description: "Argument(s) are one or more machine names.",
243-
Action: fatalOnError(cmdRegenerateCerts),
247+
Action: runCommand(cmdRegenerateCerts),
244248
Flags: []cli.Flag{
245249
cli.BoolFlag{
246250
Name: "force, f",
@@ -252,7 +256,7 @@ var Commands = []cli.Command{
252256
Name: "restart",
253257
Usage: "Restart a machine",
254258
Description: "Argument(s) are one or more machine names.",
255-
Action: fatalOnError(cmdRestart),
259+
Action: runCommand(cmdRestart),
256260
},
257261
{
258262
Flags: []cli.Flag{
@@ -268,20 +272,20 @@ var Commands = []cli.Command{
268272
Name: "rm",
269273
Usage: "Remove a machine",
270274
Description: "Argument(s) are one or more machine names.",
271-
Action: fatalOnError(cmdRm),
275+
Action: runCommand(cmdRm),
272276
},
273277
{
274278
Name: "ssh",
275279
Usage: "Log into or run a command on a machine with SSH.",
276280
Description: "Arguments are [machine-name] [command]",
277-
Action: fatalOnError(cmdSSH),
281+
Action: runCommand(cmdSSH),
278282
SkipFlagParsing: true,
279283
},
280284
{
281285
Name: "scp",
282286
Usage: "Copy files between machines",
283287
Description: "Arguments are [machine:][path] [machine:][path].",
284-
Action: fatalOnError(cmdScp),
288+
Action: runCommand(cmdScp),
285289
Flags: []cli.Flag{
286290
cli.BoolFlag{
287291
Name: "recursive, r",
@@ -293,36 +297,36 @@ var Commands = []cli.Command{
293297
Name: "start",
294298
Usage: "Start a machine",
295299
Description: "Argument(s) are one or more machine names.",
296-
Action: fatalOnError(cmdStart),
300+
Action: runCommand(cmdStart),
297301
},
298302
{
299303
Name: "status",
300304
Usage: "Get the status of a machine",
301305
Description: "Argument is a machine name.",
302-
Action: fatalOnError(cmdStatus),
306+
Action: runCommand(cmdStatus),
303307
},
304308
{
305309
Name: "stop",
306310
Usage: "Stop a machine",
307311
Description: "Argument(s) are one or more machine names.",
308-
Action: fatalOnError(cmdStop),
312+
Action: runCommand(cmdStop),
309313
},
310314
{
311315
Name: "upgrade",
312316
Usage: "Upgrade a machine to the latest version of Docker",
313317
Description: "Argument(s) are one or more machine names.",
314-
Action: fatalOnError(cmdUpgrade),
318+
Action: runCommand(cmdUpgrade),
315319
},
316320
{
317321
Name: "url",
318322
Usage: "Get the URL of a machine",
319323
Description: "Argument is a machine name.",
320-
Action: fatalOnError(cmdURL),
324+
Action: runCommand(cmdURL),
321325
},
322326
{
323327
Name: "version",
324328
Usage: "Show the Docker Machine version or a machine docker version",
325-
Action: fatalOnError(cmdVersion),
329+
Action: runCommand(cmdVersion),
326330
},
327331
}
328332

‎commands/commands_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@ package commands
22

33
import (
44
"errors"
5+
"flag"
56
"testing"
67

8+
"github.com/codegangsta/cli"
79
"github.com/docker/machine/commands/commandstest"
810
"github.com/docker/machine/drivers/fakedriver"
11+
"github.com/docker/machine/libmachine"
12+
"github.com/docker/machine/libmachine/crashreport"
913
"github.com/docker/machine/libmachine/host"
1014
"github.com/docker/machine/libmachine/hosttest"
1115
"github.com/docker/machine/libmachine/state"
@@ -131,3 +135,59 @@ func TestConsolidateError(t *testing.T) {
131135
assert.Equal(t, c.expectedErr, consolidateErrs(c.inputErrs))
132136
}
133137
}
138+
139+
type MockCrashReporter struct {
140+
sent bool
141+
}
142+
143+
func (m *MockCrashReporter) Send(err crashreport.CrashError) error {
144+
m.sent = true
145+
return nil
146+
}
147+
148+
func TestSendCrashReport(t *testing.T) {
149+
defer func(fnOsExit func(code int)) { osExit = fnOsExit }(osExit)
150+
osExit = func(code int) {}
151+
152+
defer func(factory func(baseDir string, apiKey string) crashreport.CrashReporter) {
153+
crashreport.NewCrashReporter = factory
154+
}(crashreport.NewCrashReporter)
155+
156+
tests := []struct {
157+
description string
158+
err error
159+
sent bool
160+
}{
161+
{
162+
description: "Should send crash error",
163+
err: crashreport.CrashError{
164+
Cause: errors.New("BUG"),
165+
Command: "command",
166+
Context: "context",
167+
DriverName: "virtualbox",
168+
},
169+
sent: true,
170+
},
171+
{
172+
description: "Should not send standard error",
173+
err: errors.New("BUG"),
174+
sent: false,
175+
},
176+
}
177+
178+
for _, test := range tests {
179+
mockCrashReporter := &MockCrashReporter{}
180+
crashreport.NewCrashReporter = func(baseDir string, apiKey string) crashreport.CrashReporter {
181+
return mockCrashReporter
182+
}
183+
184+
command := func(commandLine CommandLine, api libmachine.API) error {
185+
return test.err
186+
}
187+
188+
context := cli.NewContext(cli.NewApp(), &flag.FlagSet{}, nil)
189+
runCommand(command)(context)
190+
191+
assert.Equal(t, test.sent, mockCrashReporter.sent, test.description)
192+
}
193+
}

‎commands/create.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ func convertMcnFlagsToCliFlags(mcnFlags []mcnflag.Flag) ([]cli.Flag, error) {
395395
func addDriverFlagsToCommand(cliFlags []cli.Flag, cmd *cli.Command) *cli.Command {
396396
cmd.Flags = append(sharedCreateFlags, cliFlags...)
397397
cmd.SkipFlagParsing = false
398-
cmd.Action = fatalOnError(cmdCreateInner)
398+
cmd.Action = runCommand(cmdCreateInner)
399399
sort.Sort(ByFlagName(cmd.Flags))
400400

401401
return cmd

‎libmachine/crashreport/crash_report.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ type BugsnagCrashReporter struct {
4949
}
5050

5151
// NewCrashReporter creates a new bugsnag based CrashReporter. Needs an apiKey.
52-
func NewCrashReporter(baseDir string, apiKey string) *BugsnagCrashReporter {
52+
var NewCrashReporter = func(baseDir string, apiKey string) CrashReporter {
5353
if apiKey == "" {
5454
apiKey = defaultAPIKey
5555
}

0 commit comments

Comments
 (0)
X Tutup