diff --git a/internal/api/activity.go b/internal/api/activity.go index 6a3594b7..52122036 100644 --- a/internal/api/activity.go +++ b/internal/api/activity.go @@ -112,13 +112,13 @@ func (c *Client) Activity(ctx context.Context, token string, activityRequest typ b, err := c.get(ctx, url, token, "") if err != nil { - return ActivityResult{}, errHTTPRequestFailed.WithRootCause(err) + return ActivityResult{}, slackerror.New(slackerror.ErrHTTPRequestFailed).WithRootCause(err) } resp := activityResponse{} err = goutils.JSONUnmarshal(b, &resp) if err != nil { - return ActivityResult{}, errHTTPResponseInvalid.WithRootCause(err).AddAPIMethod(appActivityMethod) + return ActivityResult{}, slackerror.New(slackerror.ErrHTTPResponseInvalid).WithRootCause(err).AddAPIMethod(appActivityMethod) } if !resp.Ok { diff --git a/internal/api/client.go b/internal/api/client.go index b008a0a1..93d8ee52 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -69,9 +69,18 @@ type responseMetadata struct { } var ( - errInvalidArguments = slackerror.New(slackerror.ErrInvalidArguments) + // FIXME: errInvalidArguments should be returned as a new error from within + // the API method call to avoid changing values for this error pointer across + // API calls. + errInvalidArguments = slackerror.New(slackerror.ErrInvalidArguments) + // FIXME: errHTTPResponseInvalid should be returned as a new error right after + // the API .JSONUnmarshal function call to avoid changing values for this error + // pointer across API calls. errHTTPResponseInvalid = slackerror.New(slackerror.ErrHTTPResponseInvalid) - errHTTPRequestFailed = slackerror.New(slackerror.ErrHTTPRequestFailed) + // FIXME: errHTTPRequestFailed should be returned as a new error right after + // the API .get call methods to avoid changing values for this error pointer + // across API calls. + errHTTPRequestFailed = slackerror.New(slackerror.ErrHTTPRequestFailed) ) // NewClient accepts an httpClient to facilitate making http requests to Slack. diff --git a/internal/pkg/platform/activity.go b/internal/pkg/platform/activity.go index f089dcc4..e86eb84d 100644 --- a/internal/pkg/platform/activity.go +++ b/internal/pkg/platform/activity.go @@ -88,11 +88,12 @@ func Activity( } latestCreatedTimestamp, _, err := printLatestActivity(ctx, clients, token, activityRequest, token) - if err != nil { - return err - } + // Return an error from the activity API if the logs are not being tailed if !args.TailArg { + if err != nil { + return err + } return nil } @@ -108,9 +109,11 @@ func Activity( case <-ticker.C: // Try to grab new logs using the last logs timestamp activityRequest.MinimumDateCreated = latestCreatedTimestamp + 1 + + // Avoid exit on error from the activity API when tailing logs newLatestCreatedTimestamp, count, err := printLatestActivity(ctx, clients, token, activityRequest, token) if err != nil { - return slackerror.New(slackerror.ErrStreamingActivityLogs).WithRootCause(err) + clients.IO.PrintDebug(ctx, "%s\n", err) } if count > 0 { diff --git a/internal/pkg/platform/activity_test.go b/internal/pkg/platform/activity_test.go index d33dbc38..de8e2142 100644 --- a/internal/pkg/platform/activity_test.go +++ b/internal/pkg/platform/activity_test.go @@ -110,28 +110,80 @@ func TestPlatformActivity_StreamingLogs(t *testing.T) { }, "should return error if session validation fails": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) context.Context { - cm.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{}, slackerror.New("boomsies")) + cm.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{}, slackerror.New("mock_broken_validation")) return ctx }, - ExpectedError: slackerror.New("boomsies"), + ExpectedError: slackerror.New("mock_broken_validation"), }, - "should return error if activity request fails": { + "should return error from Activity API request if TailArg is not set": { + Args: types.ActivityArgs{ + TailArg: false, + }, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) context.Context { - cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything).Return(api.ActivityResult{}, slackerror.New("explosions")) + cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything).Return(api.ActivityResult{}, slackerror.New("mock_broken_logs")) return ctx }, - ExpectedError: slackerror.New("explosions"), + ExpectedError: slackerror.New("mock_broken_logs"), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNumberOfCalls(t, "Activity", 1) + }, }, "should return nil and invoke Activity API only once if TailArg is not set": { Args: types.ActivityArgs{ - TailArg: false, + TailArg: false, + IdleTimeoutM: 1, + PollingIntervalMS: 20, // poll activity every 20 ms }, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) context.Context { - cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything).Return(api.ActivityResult{}, nil) + cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything). + Return(api.ActivityResult{ + Activities: []api.Activity{ + { + Level: types.WARN, + ComponentID: "a123", + TraceID: "tr123", + }, + { + Level: types.ERROR, + ComponentID: "a456", + TraceID: "tr456", + }, + }, + }, nil) + ctx, cancel := context.WithCancel(ctx) + go func() { + time.Sleep(time.Millisecond * 50) // cancel activity in 50 ms + cancel() + }() return ctx }, + ExpectedError: nil, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + // with the above tail argument not included we call the method once. cm.API.AssertNumberOfCalls(t, "Activity", 1) + assert.Contains(t, cm.GetStdoutOutput(), "[warn] [a123] (Trace=tr123)") + assert.Contains(t, cm.GetStdoutOutput(), "[error] [a456] (Trace=tr456)") + }, + }, + "should return nil and invoke Activity API twice if TailArg is set while polling": { + Args: types.ActivityArgs{ + TailArg: true, + IdleTimeoutM: 0, + PollingIntervalMS: 20, // poll activity every 20 ms + }, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) context.Context { + cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything).Return(api.ActivityResult{}, nil) + ctx, cancel := context.WithCancel(ctx) + go func() { + time.Sleep(time.Millisecond * 50) // cancel activity in 50 ms + cancel() + }() + return ctx + }, + ExpectedError: nil, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + // with the above polling/canceling setup, expectation is activity called two times. + cm.API.AssertNumberOfCalls(t, "Activity", 2) }, }, "should return nil if TailArg is set and context is canceled": { @@ -148,11 +200,33 @@ func TestPlatformActivity_StreamingLogs(t *testing.T) { }() return ctx }, + ExpectedError: nil, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { // with the above polling/canceling setup, expectation is activity called only once. cm.API.AssertNumberOfCalls(t, "Activity", 1) }, }, + "should return nil if TailArg is set and activity request fails while polling": { + Args: types.ActivityArgs{ + TailArg: true, + IdleTimeoutM: 1, + PollingIntervalMS: 20, // poll activity every 20 ms + }, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) context.Context { + cm.API.On("Activity", mock.Anything, mock.Anything, mock.Anything).Return(api.ActivityResult{}, slackerror.New("mock_broken_logs")) + ctx, cancel := context.WithCancel(ctx) + go func() { + time.Sleep(time.Millisecond * 50) // cancel activity in 50 ms + cancel() + }() + return ctx + }, + ExpectedError: nil, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + // with the above polling/canceling setup, expectation is activity called three times. + cm.API.AssertNumberOfCalls(t, "Activity", 3) + }, + }, } { t.Run(name, func(t *testing.T) { // Create mocks @@ -172,6 +246,7 @@ func TestPlatformActivity_StreamingLogs(t *testing.T) { err := Activity(ctxMock, clients, &logger.Logger{}, tt.Args) if tt.ExpectedError != nil { + require.Error(t, err) assert.Contains(t, err.Error(), tt.ExpectedError.Error(), err) } else { require.NoError(t, err)