From b2a1fd450b93d876fb69e86aa65eea5886bd6e97 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 8 Nov 2019 01:32:26 +0300 Subject: [PATCH 1/4] Handle 403 Forbidden error from Mojang's API --- CHANGELOG.md | 2 ++ api/mojang/mojang.go | 11 +++++++++++ api/mojang/mojang_test.go | 21 +++++++++++++++++++++ api/mojang/queue/queue.go | 13 +++++++++---- api/mojang/queue/queue_test.go | 3 +++ 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 530d916..5d95343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] - xxxx-xx-xx +### Added +- 403 Forbidden errors from the Mojang's API are now logged ## [4.2.3] - 2019-10-03 ### Changed diff --git a/api/mojang/mojang.go b/api/mojang/mojang.go index dd4132f..14ec5d8 100644 --- a/api/mojang/mojang.go +++ b/api/mojang/mojang.go @@ -125,6 +125,8 @@ func validateResponse(response *http.Response) error { _ = json.Unmarshal(body, &decodedError) return &BadRequestError{ErrorType: decodedError.Error, Message: decodedError.Message} + case response.StatusCode == 403: + return &ForbiddenError{} case response.StatusCode == 429: return &TooManyRequestsError{} case response.StatusCode >= 500: @@ -166,6 +168,15 @@ func (*BadRequestError) IsMojangError() bool { return true } +// When Mojang decides you're such a bad guy, this error appears (even if the request has no authorization) +type ForbiddenError struct { + ResponseError +} + +func (*ForbiddenError) Error() string { + return "Forbidden" +} + // When you exceed the set limit of requests, this error will be returned type TooManyRequestsError struct { ResponseError diff --git a/api/mojang/mojang_test.go b/api/mojang/mojang_test.go index 969f7ed..2bffc81 100644 --- a/api/mojang/mojang_test.go +++ b/api/mojang/mojang_test.go @@ -102,6 +102,27 @@ func TestUsernamesToUuids(t *testing.T) { assert.Implements((*ResponseError)(nil), err) }) + t.Run("handle forbidden response", func(t *testing.T) { + assert := testify.New(t) + + defer gock.Off() + gock.New("https://api.mojang.com"). + Post("/profiles/minecraft"). + Reply(403). + BodyString("just because") + + client := &http.Client{} + gock.InterceptClient(client) + + HttpClient = client + + result, err := UsernamesToUuids([]string{"Thinkofdeath", "maksimkurb"}) + assert.Nil(result) + assert.IsType(&ForbiddenError{}, err) + assert.EqualError(err, "Forbidden") + assert.Implements((*ResponseError)(nil), err) + }) + t.Run("handle too many requests response", func(t *testing.T) { assert := testify.New(t) diff --git a/api/mojang/queue/queue.go b/api/mojang/queue/queue.go index 3eb4d8e..1aa5241 100644 --- a/api/mojang/queue/queue.go +++ b/api/mojang/queue/queue.go @@ -182,13 +182,18 @@ func (ctx *JobsQueue) handleResponseError(err error, threadName string) { switch err.(type) { case mojang.ResponseError: - if _, ok := err.(*mojang.TooManyRequestsError); ok { - ctx.Logger.Warning(":name: Got 429 Too Many Requests :err", wd.NameParam(threadName), wd.ErrParam(err)) + if _, ok := err.(*mojang.BadRequestError); ok { + ctx.Logger.Warning(":name: Got 400 Bad Request :err", wd.NameParam(threadName), wd.ErrParam(err)) return } - if _, ok := err.(*mojang.BadRequestError); ok { - ctx.Logger.Warning(":name: Got 400 Bad Request :err", wd.NameParam(threadName), wd.ErrParam(err)) + if _, ok := err.(*mojang.ForbiddenError); ok { + ctx.Logger.Warning(":name: Got 403 Forbidden :err", wd.NameParam(threadName), wd.ErrParam(err)) + return + } + + if _, ok := err.(*mojang.TooManyRequestsError); ok { + ctx.Logger.Warning(":name: Got 429 Too Many Requests :err", wd.NameParam(threadName), wd.ErrParam(err)) return } diff --git a/api/mojang/queue/queue_test.go b/api/mojang/queue/queue_test.go index 6c1e5d6..f597a69 100644 --- a/api/mojang/queue/queue_test.go +++ b/api/mojang/queue/queue_test.go @@ -403,6 +403,7 @@ func (*timeoutError) Temporary() bool { return false } var expectedErrors = []error{ &mojang.BadRequestError{}, + &mojang.ForbiddenError{}, &mojang.TooManyRequestsError{}, &mojang.ServerError{}, &timeoutError{}, @@ -418,6 +419,7 @@ func (suite *queueTestSuite) TestShouldNotLogErrorWhenExpectedErrorReturnedFromU suite.Logger.On("RecordTimer", mock.Anything, mock.Anything) suite.Logger.On("Debug", ":name: Got response error :err", mock.Anything, mock.Anything).Times(len(expectedErrors)) suite.Logger.On("Warning", ":name: Got 400 Bad Request :err", mock.Anything, mock.Anything).Once() + suite.Logger.On("Warning", ":name: Got 403 Forbidden :err", mock.Anything, mock.Anything).Once() suite.Logger.On("Warning", ":name: Got 429 Too Many Requests :err", mock.Anything, mock.Anything).Once() suite.Storage.On("GetUuid", "maksimkurb").Return("", &ValueNotFound{}) @@ -455,6 +457,7 @@ func (suite *queueTestSuite) TestShouldNotLogErrorWhenExpectedErrorReturnedFromU suite.Logger.On("RecordTimer", mock.Anything, mock.Anything) suite.Logger.On("Debug", ":name: Got response error :err", mock.Anything, mock.Anything).Times(len(expectedErrors)) suite.Logger.On("Warning", ":name: Got 400 Bad Request :err", mock.Anything, mock.Anything).Once() + suite.Logger.On("Warning", ":name: Got 403 Forbidden :err", mock.Anything, mock.Anything).Once() suite.Logger.On("Warning", ":name: Got 429 Too Many Requests :err", mock.Anything, mock.Anything).Once() suite.Storage.On("GetUuid", "maksimkurb").Return("", &ValueNotFound{}) From 7353047467b66da7b5af5455fc0d64330e8bcfe6 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 8 Nov 2019 01:54:16 +0300 Subject: [PATCH 2/4] Increase queue loop delay from 1 to 2.5 seconds. Add configuration param to adjust its value --- CHANGELOG.md | 4 ++++ README.md | 11 ++++++----- api/mojang/queue/queue.go | 7 ++++--- api/mojang/queue/queue_test.go | 2 +- cmd/serve.go | 3 +++ 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d95343..229ed22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - xxxx-xx-xx ### Added - 403 Forbidden errors from the Mojang's API are now logged +- `QUEUE_LOOP_DELAY` configuration param to adjust Mojang's textures queue performance + +### Changed +- Mojang's textures queue loop is now has an iteration delay of 2.5 seconds (was 1) ## [4.2.3] - 2019-10-03 ### Changed diff --git a/README.md b/README.md index 450a5b8..60b2b88 100644 --- a/README.md +++ b/README.md @@ -57,11 +57,12 @@ docker-compose up -d app **Variables to adjust:** -| ENV | Description | Example | -|--------------------|------------------------------------------------------------------------------------|-------------------------------------------| -| STORAGE_REDIS_POOL | By default, Chrly creates pool with 10 connection, but you may want to increase it | `20` | -| STATSD_ADDR | StatsD can be used to collect metrics | `localhost:8125` | -| SENTRY_DSN | Sentry can be used to collect app errors | `https://public:private@your.sentry.io/1` | +| ENV | Description | Example | +|--------------------|-------------------------------------------------------------------------------------------------|-------------------------------------------| +| STORAGE_REDIS_POOL | By default, Chrly creates pool with 10 connection, but you may want to increase it | `20` | +| STATSD_ADDR | StatsD can be used to collect metrics | `localhost:8125` | +| SENTRY_DSN | Sentry can be used to collect app errors | `https://public:private@your.sentry.io/1` | +| QUEUE_LOOP_DELAY | Parameter is sets the delay before each iteration of the Mojang's textures queue (milliseconds) | `3200` | If something goes wrong, you can always access logs by executing `docker-compose logs -f app`. diff --git a/api/mojang/queue/queue.go b/api/mojang/queue/queue.go index 1aa5241..7034d0b 100644 --- a/api/mojang/queue/queue.go +++ b/api/mojang/queue/queue.go @@ -14,9 +14,10 @@ import ( "github.com/elyby/chrly/api/mojang" ) +var UuidsQueueIterationDelay = 2*time.Second + 500*time.Millisecond + var usernamesToUuids = mojang.UsernamesToUuids var uuidToTextures = mojang.UuidToTextures -var uuidsQueueIterationDelay = time.Second var forever = func() bool { return true } @@ -97,13 +98,13 @@ func (ctx *JobsQueue) GetTexturesForUsername(username string) chan *mojang.Signe func (ctx *JobsQueue) startQueue() { go func() { - time.Sleep(uuidsQueueIterationDelay) + time.Sleep(UuidsQueueIterationDelay) for forever() { start := time.Now() ctx.queueRound() elapsed := time.Since(start) ctx.Logger.RecordTimer("mojang_textures.usernames.round_time", elapsed) - time.Sleep(uuidsQueueIterationDelay) + time.Sleep(UuidsQueueIterationDelay) } }() } diff --git a/api/mojang/queue/queue_test.go b/api/mojang/queue/queue_test.go index f597a69..30bc3ee 100644 --- a/api/mojang/queue/queue_test.go +++ b/api/mojang/queue/queue_test.go @@ -85,7 +85,7 @@ type queueTestSuite struct { } func (suite *queueTestSuite) SetupSuite() { - uuidsQueueIterationDelay = 0 + UuidsQueueIterationDelay = 0 } func (suite *queueTestSuite) SetupTest() { diff --git a/cmd/serve.go b/cmd/serve.go index c0c7bf9..31d137e 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "log" + "time" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -51,6 +52,7 @@ var serveCmd = &cobra.Command{ return } + queue.UuidsQueueIterationDelay = time.Duration(viper.GetInt("queue.loop_delay")) * time.Millisecond texturesStorage := queue.CreateInMemoryTexturesStorage() texturesStorage.Start() mojangTexturesQueue := &queue.JobsQueue{ @@ -86,4 +88,5 @@ func init() { viper.SetDefault("storage.redis.poll", 10) viper.SetDefault("storage.filesystem.basePath", "data") viper.SetDefault("storage.filesystem.capesDirName", "capes") + viper.SetDefault("queue.loop_delay", 2_500) } From ae185e1daabe5d557066cc49d80e47d8c30670d4 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 8 Nov 2019 02:08:10 +0300 Subject: [PATCH 3/4] Bump Go version to 1.13 --- .travis.yml | 2 +- CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 84412c2..b91a909 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ sudo: required language: go go: - - 1.12 + - 1.13 services: - docker diff --git a/CHANGELOG.md b/CHANGELOG.md index 229ed22..6cb3d03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Mojang's textures queue loop is now has an iteration delay of 2.5 seconds (was 1) +- Bumped Go version to 1.13. ## [4.2.3] - 2019-10-03 ### Changed From 6fd88e077ee17502088dab5a8d9c17ff8defa27d Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Fri, 8 Nov 2019 02:12:31 +0300 Subject: [PATCH 4/4] Prepare 4.3.0 release --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb3d03..4753aa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] - xxxx-xx-xx + +## [4.3.0] - 2019-11-08 ### Added - 403 Forbidden errors from the Mojang's API are now logged - `QUEUE_LOOP_DELAY` configuration param to adjust Mojang's textures queue performance @@ -73,7 +75,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 from the textures link instead. - `hash` field from `POST /api/skins` endpoint. -[Unreleased]: https://github.com/elyby/chrly/compare/4.2.3...HEAD +[Unreleased]: https://github.com/elyby/chrly/compare/4.3.0...HEAD +[4.3.0]: https://github.com/elyby/chrly/compare/4.2.3...4.3.0 [4.2.3]: https://github.com/elyby/chrly/compare/4.2.2...4.2.3 [4.2.2]: https://github.com/elyby/chrly/compare/4.2.1...4.2.2 [4.2.1]: https://github.com/elyby/chrly/compare/4.2.0...4.2.1