have SaveState return errors, just log them if there are other non-tr…

…ivial errors to report

cmd/snap-repair/runner.go

@@ -275,8 +275,7 @@ func (run *Runner) initState() error {
 	os.Remove(dirs.SnapRepairStateFile)
 	// TODO: init Device
 	run.stateModified = true
-	run.SaveState()
-	return nil
+	return run.SaveState()
 }
 
 // LoadState loads the repairs' state from disk, and (re)initializes it if it's missing or corrupted.
@@ -293,19 +292,20 @@ func (run *Runner) LoadState() error {
 }
 
 // SaveState saves the repairs' state to disk.
-func (run *Runner) SaveState() {
+func (run *Runner) SaveState() error {
 	if !run.stateModified {
-		return
+		return nil
 	}
 	m, err := json.Marshal(&run.state)
 	if err != nil {
-		panic(fmt.Sprintf("cannot marshal repair state: %v", err))
+		return fmt.Errorf("cannot marshal repair state: %v", err)
 	}
 	err = osutil.AtomicWriteFile(dirs.SnapRepairStateFile, m, 0600, 0)
 	if err != nil {
-		panic(fmt.Sprintf("cannot save repair state: %v", err))
+		return fmt.Errorf("cannot save repair state: %v", err)
 	}
 	run.stateModified = false
+	return nil
 }
 
 func stringList(headers map[string]interface{}, name string) ([]string, error) {
@@ -480,20 +480,27 @@ func (run *Runner) Next(brandID string) (*asserts.Repair, error) {
 	}
 	for {
 		repair, err := run.makeReady(brandID, sequenceNext)
+		// SaveState is a no-op unless makeReady modified the state
+		stateErr := run.SaveState()
+		if err != nil && err != errSkip && err != ErrRepairNotFound {
+			// err is a non trivial error, just log the SaveState error and report err
+			if stateErr != nil {
+				logger.Noticef("%v", stateErr)
+			}
+			return nil, err
+		}
+		if stateErr != nil {
+			return nil, stateErr
+		}
 		if err == ErrRepairNotFound {
-			run.SaveState()
 			return nil, ErrRepairNotFound
 		}
-		if err != nil && err != errSkip {
-			return nil, err
-		}
 
 		sequenceNext += 1
 		run.sequenceNext[brandID] = sequenceNext
 		if err == errSkip {
 			continue
 		}
-		run.SaveState()
 		return repair, nil
 	}
 }

cmd/snap-repair/runner_test.go

@@ -439,12 +439,14 @@ func (s *runnerSuite) TestSaveStateFail(c *C) {
 	defer os.Chmod(dirs.SnapRepairDir, 0775)
 
 	// no error because this is a no-op
-	runner.SaveState()
+	err = runner.SaveState()
+	c.Check(err, IsNil)
 
 	// mark as modified
 	runner.SetStateModified(true)
 
-	c.Check(runner.SaveState, PanicMatches, `cannot save repair state:.*`)
+	err = runner.SaveState()
+	c.Check(err, ErrorMatches, `cannot save repair state:.*`)
 }
 
 func (s *runnerSuite) TestSaveState(c *C) {
@@ -459,7 +461,8 @@ func (s *runnerSuite) TestSaveState(c *C) {
 	// mark as modified
 	runner.SetStateModified(true)
 
-	runner.SaveState()
+	err = runner.SaveState()
+	c.Assert(err, IsNil)
 
 	data, err := ioutil.ReadFile(dirs.SnapRepairStateFile)
 	c.Assert(err, IsNil)
@@ -744,3 +747,54 @@ AXNpZw==`
 	c.Assert(err, IsNil)
 	c.Check(string(data), Equals, `{"device":{"brand":"","model":""},"sequences":{"canonical":[{"sequence":1,"revision":1,"status":1}]}}`)
 }
+
+func (s *runnerSuite) TestNext500(c *C) {
+	restore := repair.MockPeekRetryStrategy(testRetryStrategy)
+	defer restore()
+
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(500)
+	}))
+
+	c.Assert(mockServer, NotNil)
+	defer mockServer.Close()
+
+	runner := repair.NewRunner()
+	runner.BaseURL = mustParseURL(mockServer.URL)
+	runner.LoadState()
+
+	_, err := runner.Next("canonical")
+	c.Assert(err, ErrorMatches, "cannot peek repair headers, unexpected status 500")
+}
+
+func (s *runnerSuite) TestNextSaveStateError(c *C) {
+	seqRepairs := []string{`type: repair
+authority-id: canonical
+brand-id: canonical
+repair-id: 1
+series:
+  - 33
+timestamp: 2017-07-02T12:00:00Z
+body-length: 8
+sign-key-sha3-384: KPIl7M4vQ9d4AUjkoU41TGAwtOMLc_bWUCeW8AvdRWD4_xcP60Oo4ABsFNo6BtXj
+
+scriptB
+
+
+AXNpZw==`}
+
+	mockServer := makeMockServer(c, &seqRepairs)
+	defer mockServer.Close()
+
+	runner := repair.NewRunner()
+	runner.BaseURL = mustParseURL(mockServer.URL)
+	runner.LoadState()
+
+	// break SaveState
+	err := os.Chmod(dirs.SnapRepairDir, 0555)
+	c.Assert(err, IsNil)
+	defer os.Chmod(dirs.SnapRepairDir, 0775)
+
+	_, err = runner.Next("canonical")
+	c.Check(err, ErrorMatches, `cannot save repair state:.*`)
+}