Skip to content

Commit ec326b9

Browse files
teranteran
authored
Fix 5xx errors on underlying error appearance with cache turned on (#115)
Signed-off-by: Igor Shishkin <[email protected]>
1 parent 3628cc0 commit ec326b9

File tree

3 files changed

+115
-31
lines changed

3 files changed

+115
-31
lines changed

‎cmd/publisher/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func main() {
101101
panic(err)
102102
}
103103

104-
repo = memcache.New(cli, repo, cfg.MemcacheTTL)
104+
repo = memcache.New(cli, repo, cfg.MemcacheTTL, "publisher")
105105
}
106106

107107
awsSession, err := session.NewSession(&aws.Config{

‎repositories/cache/metadata/memcache/memcache.go‎

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"time"
99

1010
memcacheCli "github.com/bradfitz/gomemcache/memcache"
11-
"github.com/pkg/errors"
1211
log "github.com/sirupsen/logrus"
1312

1413
emodels "github.com/teran/archived/exporter/models"
@@ -19,16 +18,22 @@ import (
1918
var _ metadata.Repository = (*memcache)(nil)
2019

2120
type memcache struct {
22-
cli *memcacheCli.Client
23-
repo metadata.Repository
24-
ttl time.Duration
21+
cli *memcacheCli.Client
22+
keyPrefix string
23+
repo metadata.Repository
24+
ttl time.Duration
2525
}
2626

27-
func New(cli *memcacheCli.Client, repo metadata.Repository, ttl time.Duration) metadata.Repository {
27+
func New(cli *memcacheCli.Client, repo metadata.Repository, ttl time.Duration, keyPrefix string) metadata.Repository {
28+
if keyPrefix == "" {
29+
keyPrefix = "_"
30+
}
31+
2832
return &memcache{
29-
cli: cli,
30-
repo: repo,
31-
ttl: ttl,
33+
cli: cli,
34+
keyPrefix: keyPrefix,
35+
repo: repo,
36+
ttl: ttl,
3237
}
3338
}
3439

@@ -37,7 +42,11 @@ func (m *memcache) CreateContainer(ctx context.Context, name string) error {
3742
}
3843

3944
func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {
40-
cacheKey := "_ListContainers"
45+
cacheKey := strings.Join([]string{
46+
m.keyPrefix,
47+
"ListContainers",
48+
}, ":")
49+
4150
item, err := m.cli.Get(cacheKey)
4251
if err != nil {
4352
if err == memcacheCli.ErrCacheMiss {
@@ -47,7 +56,7 @@ func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {
4756

4857
containers, err := m.repo.ListContainers(ctx)
4958
if err != nil {
50-
return nil, errors.Wrap(err, "error retrieving container list")
59+
return nil, err
5160
}
5261

5362
if err := store(m, cacheKey, containers); err != nil {
@@ -66,7 +75,7 @@ func (m *memcache) ListContainers(ctx context.Context) ([]string, error) {
6675
var retrievedValue []string
6776
err = json.Unmarshal(item.Value, &retrievedValue)
6877
if err != nil {
69-
return nil, errors.Wrap(err, "error unmarshaling cached value")
78+
return nil, err
7079
}
7180

7281
return retrievedValue, nil
@@ -81,7 +90,12 @@ func (m *memcache) CreateVersion(ctx context.Context, container string) (string,
8190
}
8291

8392
func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, container string) (string, error) {
84-
cacheKey := "_GetLatestPublishedVersionByContainer:" + container
93+
cacheKey := strings.Join([]string{
94+
m.keyPrefix,
95+
"GetLatestPublishedVersionByContainer",
96+
container,
97+
}, ":")
98+
8599
item, err := m.cli.Get(cacheKey)
86100
if err != nil {
87101
if err == memcacheCli.ErrCacheMiss {
@@ -91,7 +105,7 @@ func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, con
91105

92106
version, err := m.repo.GetLatestPublishedVersionByContainer(ctx, container)
93107
if err != nil {
94-
return "", errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
108+
return "", err
95109
}
96110

97111
if err := store(m, cacheKey, version); err != nil {
@@ -110,14 +124,19 @@ func (m *memcache) GetLatestPublishedVersionByContainer(ctx context.Context, con
110124
var retrievedValue string
111125
err = json.Unmarshal(item.Value, &retrievedValue)
112126
if err != nil {
113-
return "", errors.Wrap(err, "error unmarshaling cached value")
127+
return "", err
114128
}
115129

116130
return retrievedValue, nil
117131
}
118132

119133
func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container string) ([]models.Version, error) {
120-
cacheKey := "_ListAllVersionsByContainer:" + container
134+
cacheKey := strings.Join([]string{
135+
m.keyPrefix,
136+
"ListAllVersionsByContainer",
137+
container,
138+
}, ":")
139+
121140
item, err := m.cli.Get(cacheKey)
122141
if err != nil {
123142
if err == memcacheCli.ErrCacheMiss {
@@ -127,7 +146,7 @@ func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container str
127146

128147
versions, err := m.repo.ListAllVersionsByContainer(ctx, container)
129148
if err != nil {
130-
return nil, errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
149+
return nil, err
131150
}
132151

133152
if err := store(m, cacheKey, versions); err != nil {
@@ -146,14 +165,19 @@ func (m *memcache) ListAllVersionsByContainer(ctx context.Context, container str
146165
var retrievedValue []models.Version
147166
err = json.Unmarshal(item.Value, &retrievedValue)
148167
if err != nil {
149-
return nil, errors.Wrap(err, "error unmarshaling cached value")
168+
return nil, err
150169
}
151170

152171
return retrievedValue, nil
153172
}
154173

155174
func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, container string) ([]models.Version, error) {
156-
cacheKey := "_ListPublishedVersionsByContainer:" + container
175+
cacheKey := strings.Join([]string{
176+
m.keyPrefix,
177+
"ListPublishedVersionsByContainer",
178+
container,
179+
}, ":")
180+
157181
item, err := m.cli.Get(cacheKey)
158182
if err != nil {
159183
if err == memcacheCli.ErrCacheMiss {
@@ -163,7 +187,7 @@ func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, contain
163187

164188
versions, err := m.repo.ListPublishedVersionsByContainer(ctx, container)
165189
if err != nil {
166-
return nil, errors.Wrapf(err, "error retrieving latest version for container `%s`", container)
190+
return nil, err
167191
}
168192

169193
if err := store(m, cacheKey, versions); err != nil {
@@ -182,7 +206,7 @@ func (m *memcache) ListPublishedVersionsByContainer(ctx context.Context, contain
182206
var retrievedValue []models.Version
183207
err = json.Unmarshal(item.Value, &retrievedValue)
184208
if err != nil {
185-
return nil, errors.Wrap(err, "error unmarshaling cached value")
209+
return nil, err
186210
}
187211

188212
return retrievedValue, nil
@@ -195,7 +219,8 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,
195219
}
196220

197221
cacheKey := strings.Join([]string{
198-
"_ListPublishedVersionsByContainerAndPage",
222+
m.keyPrefix,
223+
"ListPublishedVersionsByContainerAndPage",
199224
container,
200225
strconv.FormatUint(offset, 10),
201226
strconv.FormatUint(limit, 10),
@@ -210,7 +235,7 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,
210235

211236
n, versions, err := m.repo.ListPublishedVersionsByContainerAndPage(ctx, container, offset, limit)
212237
if err != nil {
213-
return 0, nil, errors.Wrapf(err, "error retrieving published versions for container `%s` (offset=%d; limit=%d)", container, offset, limit)
238+
return 0, nil, err
214239
}
215240

216241
if err := store(m, cacheKey, proxy{Total: n, Versions: versions}); err != nil {
@@ -229,7 +254,7 @@ func (m *memcache) ListPublishedVersionsByContainerAndPage(ctx context.Context,
229254
var retrievedValue proxy
230255
err = json.Unmarshal(item.Value, &retrievedValue)
231256
if err != nil {
232-
return 0, nil, errors.Wrap(err, "error unmarshaling cached value")
257+
return 0, nil, err
233258
}
234259

235260
return retrievedValue.Total, retrievedValue.Versions, nil
@@ -258,7 +283,8 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o
258283
}
259284

260285
cacheKey := strings.Join([]string{
261-
"_ListObjects",
286+
m.keyPrefix,
287+
"ListObjects",
262288
container,
263289
version,
264290
strconv.FormatUint(offset, 10),
@@ -274,7 +300,7 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o
274300

275301
n, objects, err := m.repo.ListObjects(ctx, container, version, offset, limit)
276302
if err != nil {
277-
return 0, nil, errors.Wrapf(err, "error retrieving objects for `%s/%s`, offset=%d; limit=%d", container, version, offset, limit)
303+
return 0, nil, err
278304
}
279305

280306
if err := store(m, cacheKey, proxy{Total: n, Objects: objects}); err != nil {
@@ -293,7 +319,7 @@ func (m *memcache) ListObjects(ctx context.Context, container, version string, o
293319
var retrievedValue proxy
294320
err = json.Unmarshal(item.Value, &retrievedValue)
295321
if err != nil {
296-
return 0, nil, errors.Wrap(err, "error unmarshaling cached value")
322+
return 0, nil, err
297323
}
298324

299325
return retrievedValue.Total, retrievedValue.Objects, nil
@@ -313,7 +339,8 @@ func (m *memcache) CreateBLOB(ctx context.Context, checksum string, size uint64,
313339

314340
func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, key string) (string, error) {
315341
cacheKey := strings.Join([]string{
316-
"_GetBlobKeyByObject",
342+
m.keyPrefix,
343+
"GetBlobKeyByObject",
317344
container,
318345
version,
319346
key,
@@ -328,7 +355,7 @@ func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, k
328355

329356
key, err := m.repo.GetBlobKeyByObject(ctx, container, version, key)
330357
if err != nil {
331-
return "", errors.Wrapf(err, "error retrieving object key for `%s/%s/%s`", container, version, key)
358+
return "", err
332359
}
333360

334361
if err = store(m, cacheKey, key); err != nil {
@@ -347,7 +374,7 @@ func (m *memcache) GetBlobKeyByObject(ctx context.Context, container, version, k
347374
var retrievedValue string
348375
err = json.Unmarshal(item.Value, &retrievedValue)
349376
if err != nil {
350-
return "", errors.Wrap(err, "error unmarshaling cached value")
377+
return "", err
351378
}
352379

353380
return retrievedValue, nil

‎repositories/cache/metadata/memcache/memcache_test.go‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"time"
77

88
memcacheCli "github.com/bradfitz/gomemcache/memcache"
9+
"github.com/pkg/errors"
910
log "github.com/sirupsen/logrus"
1011
"github.com/stretchr/testify/suite"
1112

@@ -33,6 +34,14 @@ func (s *memcacheTestSuite) TestListContainers() {
3334
s.Require().Equal([]string{"container1"}, containers)
3435
}
3536

37+
func (s *memcacheTestSuite) TestListContainersError() {
38+
s.repoMock.On("ListContainers").Return([]string{}, errors.New("some error")).Once()
39+
40+
_, err := s.cache.ListContainers(s.ctx)
41+
s.Require().Error(err)
42+
s.Require().Equal("some error", err.Error())
43+
}
44+
3645
func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainer() {
3746
s.repoMock.On("GetLatestPublishedVersionByContainer", "test-container").Return("test-version", nil).Once()
3847

@@ -45,6 +54,14 @@ func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainer() {
4554
s.Require().Equal("test-version", version)
4655
}
4756

57+
func (s *memcacheTestSuite) TestGetLatestPublishedVersionByContainerError() {
58+
s.repoMock.On("GetLatestPublishedVersionByContainer", "test-container").Return("", errors.New("some error")).Once()
59+
60+
_, err := s.cache.GetLatestPublishedVersionByContainer(s.ctx, "test-container")
61+
s.Require().Error(err)
62+
s.Require().Equal("some error", err.Error())
63+
}
64+
4865
func (s *memcacheTestSuite) TestListAllVersionsByContainer() {
4966
s.repoMock.On("ListAllVersionsByContainer", "test-container").Return([]models.Version{{Name: "test-version"}}, nil).Once()
5067

@@ -57,6 +74,14 @@ func (s *memcacheTestSuite) TestListAllVersionsByContainer() {
5774
s.Require().Equal([]models.Version{{Name: "test-version"}}, versions)
5875
}
5976

77+
func (s *memcacheTestSuite) TestListAllVersionsByContainerError() {
78+
s.repoMock.On("ListAllVersionsByContainer", "test-container").Return([]models.Version{}, errors.New("some error")).Once()
79+
80+
_, err := s.cache.ListAllVersionsByContainer(s.ctx, "test-container")
81+
s.Require().Error(err)
82+
s.Require().Equal("some error", err.Error())
83+
}
84+
6085
func (s *memcacheTestSuite) TestListPublishedVersionsByContainer() {
6186
s.repoMock.On("ListPublishedVersionsByContainer", "test-container").Return([]models.Version{{Name: "test-version"}}, nil).Once()
6287

@@ -69,6 +94,14 @@ func (s *memcacheTestSuite) TestListPublishedVersionsByContainer() {
6994
s.Require().Equal([]models.Version{{Name: "test-version"}}, versions)
7095
}
7196

97+
func (s *memcacheTestSuite) TestListPublishedVersionsByContainerError() {
98+
s.repoMock.On("ListPublishedVersionsByContainer", "test-container").Return([]models.Version{}, errors.New("some error")).Once()
99+
100+
_, err := s.cache.ListPublishedVersionsByContainer(s.ctx, "test-container")
101+
s.Require().Error(err)
102+
s.Require().Equal("some error", err.Error())
103+
}
104+
72105
func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPage() {
73106
s.repoMock.On("ListPublishedVersionsByContainerAndPage", "test-container", uint64(0), uint64(15)).Return(uint64(500), []models.Version{{Name: "test-version"}}, nil).Once()
74107

@@ -83,6 +116,14 @@ func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPage() {
83116
s.Require().Equal(uint64(500), total)
84117
}
85118

119+
func (s *memcacheTestSuite) TestListPublishedVersionsByContainerAndPageError() {
120+
s.repoMock.On("ListPublishedVersionsByContainerAndPage", "test-container", uint64(0), uint64(15)).Return(uint64(0), []models.Version{}, errors.New("some error")).Once()
121+
122+
_, _, err := s.cache.ListPublishedVersionsByContainerAndPage(s.ctx, "test-container", 0, 15)
123+
s.Require().Error(err)
124+
s.Require().Equal("some error", err.Error())
125+
}
126+
86127
func (s *memcacheTestSuite) TestListObjects() {
87128
s.repoMock.On("ListObjects", "test-container", "test-version", uint64(0), uint64(30)).Return(uint64(500), []string{"obj1"}, nil).Once()
88129

@@ -97,6 +138,14 @@ func (s *memcacheTestSuite) TestListObjects() {
97138
s.Require().Equal(uint64(500), total)
98139
}
99140

141+
func (s *memcacheTestSuite) TestListObjectsError() {
142+
s.repoMock.On("ListObjects", "test-container", "test-version", uint64(0), uint64(30)).Return(uint64(0), []string{}, errors.New("some error")).Once()
143+
144+
_, _, err := s.cache.ListObjects(s.ctx, "test-container", "test-version", 0, 30)
145+
s.Require().Error(err)
146+
s.Require().Equal("some error", err.Error())
147+
}
148+
100149
func (s *memcacheTestSuite) TestGetBlobKeyByObject() {
101150
s.repoMock.On("GetBlobKeyByObject", "container", "version", "key").Return("deadbeef", nil).Once()
102151

@@ -109,6 +158,14 @@ func (s *memcacheTestSuite) TestGetBlobKeyByObject() {
109158
s.Require().Equal("deadbeef", casKey)
110159
}
111160

161+
func (s *memcacheTestSuite) TestGetBlobKeyByObjectError() {
162+
s.repoMock.On("GetBlobKeyByObject", "container", "version", "key").Return("", errors.New("some error")).Once()
163+
164+
_, err := s.cache.GetBlobKeyByObject(s.ctx, "container", "version", "key")
165+
s.Require().Error(err)
166+
s.Require().Equal("some error", err.Error())
167+
}
168+
112169
// Non-cached methods ...
113170
func (s *memcacheTestSuite) TestCreateContainer() {
114171
s.repoMock.On("CreateContainer", "container1").Return(nil).Twice()
@@ -264,7 +321,7 @@ func (s *memcacheTestSuite) SetupTest() {
264321
s.Require().NoError(err)
265322

266323
cli := memcacheCli.New(url)
267-
s.cache = New(cli, s.repoMock, 3*time.Second)
324+
s.cache = New(cli, s.repoMock, 3*time.Second, s.T().Name())
268325
}
269326

270327
func (s *memcacheTestSuite) TearDownTest() {

0 commit comments

Comments
 (0)