From 43489c2b676fc68067e4555935305a4156aa6042 Mon Sep 17 00:00:00 2001
From: Wenkai Yin <yinw@vmware.com>
Date: Thu, 14 Dec 2017 13:02:11 +0800
Subject: [PATCH] Print stack trace when recover from panic and print warning
 message rather than returning an error when updating 0 records

---
 src/common/dao/clair.go            |  8 ++--
 src/common/dao/dao_test.go         | 63 ++++++++++++++----------------
 src/common/dao/replication_job.go  |  7 ++--
 src/common/dao/scan_job.go         |  6 ++-
 src/jobservice/job/statemachine.go |  3 +-
 5 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/src/common/dao/clair.go b/src/common/dao/clair.go
index 170a038e8..b978566d2 100644
--- a/src/common/dao/clair.go
+++ b/src/common/dao/clair.go
@@ -16,8 +16,8 @@ package dao
 
 import (
 	"github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/common/utils/log"
 
-	"fmt"
 	"time"
 )
 
@@ -35,12 +35,10 @@ func SetClairVulnTimestamp(namespace string, timestamp time.Time) error {
 	if !created {
 		rec.LastUpdate = timestamp
 		n, err := o.Update(rec)
-		if err != nil {
-			return err
-		}
 		if n == 0 {
-			return fmt.Errorf("No record is updated, record: %v", *rec)
+			log.Warningf("no records are updated for %v", *rec)
 		}
+		return err
 	}
 	return nil
 }
diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go
index bb0c2ce20..26c7e1965 100644
--- a/src/common/dao/dao_test.go
+++ b/src/common/dao/dao_test.go
@@ -20,8 +20,8 @@ import (
 	"time"
 
 	"github.com/astaxie/beego/orm"
-	"github.com/vmware/harbor/src/common"
 	"github.com/stretchr/testify/assert"
+	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/models"
 	"github.com/vmware/harbor/src/common/utils"
 	"github.com/vmware/harbor/src/common/utils/log"
@@ -1531,8 +1531,6 @@ func TestUpdateScanJobStatus(t *testing.T) {
 	j, err := GetScanJob(id)
 	assert.Nil(err)
 	assert.Equal("newstatus", j.Status)
-	err = UpdateScanJobStatus(id+9, "newstatus")
-	assert.NotNil(err)
 	err = ClearTable(models.ScanJobTable)
 	assert.Nil(err)
 }
@@ -1632,36 +1630,35 @@ func TestGetScanJobsByStatus(t *testing.T) {
 	assert.Equal(sj1.Repository, r2[0].Repository)
 }
 
-
-func TestSaveConfigEntries(t *testing.T)  {
-	configEntries :=[]models.ConfigEntry{
+func TestSaveConfigEntries(t *testing.T) {
+	configEntries := []models.ConfigEntry{
 		{
-			Key:"teststringkey",
-			Value:"192.168.111.211",
+			Key:   "teststringkey",
+			Value: "192.168.111.211",
 		},
 		{
-			Key:"testboolkey",
-			Value:"true",
+			Key:   "testboolkey",
+			Value: "true",
 		},
 		{
-			Key:"testnumberkey",
-			Value:"5",
+			Key:   "testnumberkey",
+			Value: "5",
 		},
 		{
-			Key:common.CfgDriverDB,
-			Value:"db",
+			Key:   common.CfgDriverDB,
+			Value: "db",
 		},
 	}
 	err := SaveConfigEntries(configEntries)
 	if err != nil {
 		t.Fatalf("failed to save configuration to database %v", err)
 	}
-	readEntries, err:=GetConfigEntries()
-	if err !=nil {
+	readEntries, err := GetConfigEntries()
+	if err != nil {
 		t.Fatalf("Failed to get configuration from database %v", err)
 	}
-	findItem:=0
-	for _,entry:= range readEntries{
+	findItem := 0
+	for _, entry := range readEntries {
 		switch entry.Key {
 		case "teststringkey":
 			if "192.168.111.211" == entry.Value {
@@ -1678,38 +1675,38 @@ func TestSaveConfigEntries(t *testing.T)  {
 		default:
 		}
 	}
-	if findItem !=3 {
+	if findItem != 3 {
 		t.Fatalf("Should update 3 configuration but only update %d", findItem)
 	}
 
-	configEntries =[]models.ConfigEntry{
+	configEntries = []models.ConfigEntry{
 		{
-			Key:"teststringkey",
-			Value:"192.168.111.215",
+			Key:   "teststringkey",
+			Value: "192.168.111.215",
 		},
 		{
-			Key:"testboolkey",
-			Value:"false",
+			Key:   "testboolkey",
+			Value: "false",
 		},
 		{
-			Key:"testnumberkey",
-			Value:"7",
+			Key:   "testnumberkey",
+			Value: "7",
 		},
 		{
-			Key:common.CfgDriverDB,
-			Value:"db",
+			Key:   common.CfgDriverDB,
+			Value: "db",
 		},
 	}
 	err = SaveConfigEntries(configEntries)
 	if err != nil {
 		t.Fatalf("failed to save configuration to database %v", err)
 	}
-	readEntries, err=GetConfigEntries()
-	if err !=nil {
+	readEntries, err = GetConfigEntries()
+	if err != nil {
 		t.Fatalf("Failed to get configuration from database %v", err)
 	}
-	findItem=0
-	for _,entry:= range readEntries{
+	findItem = 0
+	for _, entry := range readEntries {
 		switch entry.Key {
 		case "teststringkey":
 			if "192.168.111.215" == entry.Value {
@@ -1726,7 +1723,7 @@ func TestSaveConfigEntries(t *testing.T)  {
 		default:
 		}
 	}
-	if findItem !=3 {
+	if findItem != 3 {
 		t.Fatalf("Should update 3 configuration but only update %d", findItem)
 	}
 
diff --git a/src/common/dao/replication_job.go b/src/common/dao/replication_job.go
index eb6593259..b53fcfd08 100644
--- a/src/common/dao/replication_job.go
+++ b/src/common/dao/replication_job.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/astaxie/beego/orm"
 	"github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/common/utils/log"
 )
 
 // AddRepTarget ...
@@ -397,9 +398,9 @@ func UpdateRepJobStatus(id int64, status string) error {
 		Status:     status,
 		UpdateTime: time.Now(),
 	}
-	num, err := o.Update(&j, "Status", "UpdateTime")
-	if num == 0 {
-		err = fmt.Errorf("Failed to update replication job with id: %d %s", id, err.Error())
+	n, err := o.Update(&j, "Status", "UpdateTime")
+	if n == 0 {
+		log.Warningf("no records are updated when updating replication job %d", id)
 	}
 	return err
 }
diff --git a/src/common/dao/scan_job.go b/src/common/dao/scan_job.go
index 2fd666fe1..ae62ddc9a 100644
--- a/src/common/dao/scan_job.go
+++ b/src/common/dao/scan_job.go
@@ -17,6 +17,7 @@ package dao
 import (
 	"github.com/astaxie/beego/orm"
 	"github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/common/utils/log"
 
 	"encoding/json"
 	"fmt"
@@ -78,7 +79,7 @@ func UpdateScanJobStatus(id int64, status string) error {
 	}
 	n, err := o.Update(&sj, "Status", "UpdateTime")
 	if n == 0 {
-		return fmt.Errorf("Failed to update scan job with id: %d, error: %v", id, err)
+		log.Warningf("no records are updated when updating scan job %d", id)
 	}
 	return err
 }
@@ -109,8 +110,9 @@ func SetScanJobForImg(digest string, jobID int64) error {
 		rec.UpdateTime = time.Now()
 		n, err := o.Update(rec, "JobID", "UpdateTime")
 		if n == 0 {
-			return fmt.Errorf("Failed to set scan job for image with digest: %s, error: %v", digest, err)
+			log.Warningf("no records are updated when setting scan job for image with digest %s", digest)
 		}
+		return err
 	}
 	return nil
 }
diff --git a/src/jobservice/job/statemachine.go b/src/jobservice/job/statemachine.go
index 9bf946451..3db88a78f 100644
--- a/src/jobservice/job/statemachine.go
+++ b/src/jobservice/job/statemachine.go
@@ -16,6 +16,7 @@ package job
 
 import (
 	"fmt"
+	"runtime/debug"
 	"sync"
 
 	"github.com/vmware/harbor/src/common/models"
@@ -80,7 +81,7 @@ func (sm *SM) Start(s string) {
 	defer func() {
 		if r := recover(); r != nil {
 			sm.Logger.Errorf("Panic: %v, entering error state", r)
-			log.Warningf("Panic when handling job: %v, panic: %v, entering error state", sm.CurrentJob, r)
+			log.Warningf("Panic when handling job: %v, panic: %v \n %s \n, entering error state", sm.CurrentJob, r, debug.Stack())
 			sm.EnterState(models.JobError)
 		}
 	}()