From d8961577beefe29462b68b22dbb675c32bf7e1dd Mon Sep 17 00:00:00 2001
From: Michael Snowden <MichaelSnowden@users.noreply.github.com>
Date: Wed, 21 Dec 2022 12:20:01 -0800
Subject: [PATCH] Fix errcheck in ./common/persistence/ (#3743)

---
 common/persistence/dataInterfaces.go    |  1 +
 common/persistence/sql/common.go        |  5 ++++-
 common/persistence/tests/sqlite_test.go | 21 ++++++++++++++++-----
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/common/persistence/dataInterfaces.go b/common/persistence/dataInterfaces.go
index a155b8271d5..2b49b6aef55 100644
--- a/common/persistence/dataInterfaces.go
+++ b/common/persistence/dataInterfaces.go
@@ -1044,6 +1044,7 @@ type (
 	}
 
 	// Closeable is an interface for any entity that supports a close operation to release resources
+	// TODO: allow this method to return errors
 	Closeable interface {
 		Close()
 	}
diff --git a/common/persistence/sql/common.go b/common/persistence/sql/common.go
index 1dc5f085655..c24a943cb94 100644
--- a/common/persistence/sql/common.go
+++ b/common/persistence/sql/common.go
@@ -59,7 +59,10 @@ func (m *SqlStore) GetName() string {
 
 func (m *SqlStore) Close() {
 	if m.Db != nil {
-		m.Db.Close()
+		err := m.Db.Close()
+		if err != nil {
+			m.logger.Error("Error closing SQL database", tag.Error(err))
+		}
 	}
 }
 
diff --git a/common/persistence/tests/sqlite_test.go b/common/persistence/tests/sqlite_test.go
index 758c4ea4b0b..5745c81ac5a 100644
--- a/common/persistence/tests/sqlite_test.go
+++ b/common/persistence/tests/sqlite_test.go
@@ -30,6 +30,7 @@ import (
 	"path"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 
 	"go.temporal.io/server/common/config"
@@ -179,7 +180,9 @@ func TestSQLiteTaskQueueTaskSuite(t *testing.T) {
 func TestSQLiteFileExecutionMutableStateStoreSuite(t *testing.T) {
 	cfg := NewSQLiteFileConfig()
 	SetupSQLiteDatabase(cfg)
-	defer os.Remove(cfg.DatabaseName)
+	defer func() {
+		assert.NoError(t, os.Remove(cfg.DatabaseName))
+	}()
 	logger := log.NewNoopLogger()
 	factory := sql.NewFactory(
 		*cfg,
@@ -212,7 +215,9 @@ func TestSQLiteFileExecutionMutableStateStoreSuite(t *testing.T) {
 func TestSQLiteFileExecutionMutableStateTaskStoreSuite(t *testing.T) {
 	cfg := NewSQLiteFileConfig()
 	SetupSQLiteDatabase(cfg)
-	defer os.Remove(cfg.DatabaseName)
+	defer func() {
+		assert.NoError(t, os.Remove(cfg.DatabaseName))
+	}()
 	logger := log.NewNoopLogger()
 	factory := sql.NewFactory(
 		*cfg,
@@ -245,7 +250,9 @@ func TestSQLiteFileExecutionMutableStateTaskStoreSuite(t *testing.T) {
 func TestSQLiteFileHistoryStoreSuite(t *testing.T) {
 	cfg := NewSQLiteFileConfig()
 	SetupSQLiteDatabase(cfg)
-	defer os.Remove(cfg.DatabaseName)
+	defer func() {
+		assert.NoError(t, os.Remove(cfg.DatabaseName))
+	}()
 	logger := log.NewNoopLogger()
 	factory := sql.NewFactory(
 		*cfg,
@@ -268,7 +275,9 @@ func TestSQLiteFileHistoryStoreSuite(t *testing.T) {
 func TestSQLiteFileTaskQueueSuite(t *testing.T) {
 	cfg := NewSQLiteFileConfig()
 	SetupSQLiteDatabase(cfg)
-	defer os.Remove(cfg.DatabaseName)
+	defer func() {
+		assert.NoError(t, os.Remove(cfg.DatabaseName))
+	}()
 	logger := log.NewNoopLogger()
 	factory := sql.NewFactory(
 		*cfg,
@@ -291,7 +300,9 @@ func TestSQLiteFileTaskQueueSuite(t *testing.T) {
 func TestSQLiteFileTaskQueueTaskSuite(t *testing.T) {
 	cfg := NewSQLiteFileConfig()
 	SetupSQLiteDatabase(cfg)
-	defer os.Remove(cfg.DatabaseName)
+	defer func() {
+		assert.NoError(t, os.Remove(cfg.DatabaseName))
+	}()
 	logger := log.NewNoopLogger()
 	factory := sql.NewFactory(
 		*cfg,