Skip to content

Commit 1ab5496

Browse files
authored
refactor: remove internal/logger from platform run (#378)
* refactor: remove logger from activity, validate, datastore, auth list, delete, and uninstall * refactor: remove logger from project create * refactor: remove logger from app install, add, and function mocks * refactor: remove logger from platform run * fix: correct merge conflict resolutions for create.go and install.go
1 parent 2ae86a3 commit 1ab5496

5 files changed

Lines changed: 40 additions & 119 deletions

File tree

cmd/platform/run.go

Lines changed: 1 addition & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,10 @@ import (
2121
"github.com/slackapi/slack-cli/cmd/triggers"
2222
internalapp "github.com/slackapi/slack-cli/internal/app"
2323
"github.com/slackapi/slack-cli/internal/cmdutil"
24-
"github.com/slackapi/slack-cli/internal/logger"
2524
"github.com/slackapi/slack-cli/internal/pkg/platform"
2625
"github.com/slackapi/slack-cli/internal/prompts"
2726
"github.com/slackapi/slack-cli/internal/shared"
2827
"github.com/slackapi/slack-cli/internal/slackerror"
29-
"github.com/slackapi/slack-cli/internal/slacktrace"
3028
"github.com/slackapi/slack-cli/internal/style"
3129
"github.com/spf13/cobra"
3230
)
@@ -144,68 +142,10 @@ func RunRunCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []str
144142
OrgGrantWorkspaceID: runFlags.orgGrantWorkspaceID,
145143
}
146144

147-
log := newRunLogger(clients, cmd)
148-
149145
// Run dev app locally
150-
if _, _, err := runFunc(ctx, clients, log, runArgs); err != nil {
146+
if _, err := runFunc(ctx, clients, runArgs); err != nil {
151147
return err
152148
}
153149

154150
return nil
155151
}
156-
157-
// newRunLogger creates a logger instance to receive event notifications
158-
func newRunLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Logger {
159-
ctx := cmd.Context()
160-
return logger.New(
161-
// OnEvent
162-
func(event *logger.LogEvent) {
163-
switch event.Name {
164-
case "on_update_app_install":
165-
cmd.Println(style.Secondary(fmt.Sprintf(
166-
`Updating local app install for "%s"`,
167-
event.DataToString("teamName"),
168-
)))
169-
case "on_cloud_run_connection_connected":
170-
clients.IO.PrintTrace(ctx, slacktrace.PlatformRunReady)
171-
cmd.Println(style.Secondary("Connected, awaiting events"))
172-
case "on_cloud_run_connection_message":
173-
message := event.DataToString("cloud_run_connection_message")
174-
clients.IO.PrintDebug(ctx, "received: %s", message)
175-
case "on_cloud_run_connection_command_error":
176-
message := event.DataToString("cloud_run_connection_command_error")
177-
clients.IO.PrintError(ctx, "Error: %s", message)
178-
case "on_cloud_run_watch_error":
179-
message := event.DataToString("cloud_run_watch_error")
180-
clients.IO.PrintError(ctx, "Error: %s", message)
181-
case "on_cloud_run_watch_manifest_change":
182-
path := event.DataToString("cloud_run_watch_manifest_change")
183-
cmd.Println(style.Secondary(fmt.Sprintf("Manifest change detected: %s, reinstalling app...", path)))
184-
case "on_cloud_run_watch_manifest_change_reinstalled":
185-
cmd.Println(style.Secondary("App successfully reinstalled"))
186-
case "on_cloud_run_watch_manifest_change_skipped_remote":
187-
path := event.DataToString("cloud_run_watch_manifest_change_skipped")
188-
cmd.Println(style.Secondary(fmt.Sprintf("Manifest change detected: %s, skipped reinstalling app because manifest.source=remote", path)))
189-
case "on_cloud_run_watch_app_change":
190-
path := event.DataToString("cloud_run_watch_app_change")
191-
cmd.Println(style.Secondary(fmt.Sprintf("App change detected: %s, restarting server...", path)))
192-
case "on_cleanup_app_install_done":
193-
cmd.Println(style.Secondary(fmt.Sprintf(
194-
`Cleaned up local app install for "%s".`,
195-
event.DataToString("teamName"),
196-
)))
197-
case "on_cleanup_app_install_failed":
198-
cmd.Println(style.Secondary(fmt.Sprintf(
199-
`Cleaning up local app install for "%s" failed.`,
200-
event.DataToString("teamName"),
201-
)))
202-
message := event.DataToString("on_cleanup_app_install_error")
203-
clients.IO.PrintWarning(ctx, "Local app cleanup failed: %s", message)
204-
case "on_abort_cleanup_app_install":
205-
cmd.Println(style.Secondary("Aborting, local app might not be cleaned up."))
206-
default:
207-
// Ignore the event
208-
}
209-
},
210-
)
211-
}

cmd/platform/run_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/slackapi/slack-cli/internal/cmdutil"
2222
"github.com/slackapi/slack-cli/internal/hooks"
23-
"github.com/slackapi/slack-cli/internal/logger"
2423
"github.com/slackapi/slack-cli/internal/pkg/platform"
2524
"github.com/slackapi/slack-cli/internal/prompts"
2625
"github.com/slackapi/slack-cli/internal/shared"
@@ -48,9 +47,9 @@ type RunPkgMock struct {
4847
mock.Mock
4948
}
5049

51-
func (m *RunPkgMock) Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, runArgs platform.RunArgs) (*logger.LogEvent, types.InstallState, error) {
52-
args := m.Called(ctx, clients, log, runArgs)
53-
return log.SuccessEvent(), types.InstallSuccess, args.Error(0)
50+
func (m *RunPkgMock) Run(ctx context.Context, clients *shared.ClientFactory, runArgs platform.RunArgs) (types.InstallState, error) {
51+
args := m.Called(ctx, clients, runArgs)
52+
return types.InstallSuccess, args.Error(0)
5453
}
5554

5655
func TestRunCommand_Flags(t *testing.T) {
@@ -222,7 +221,7 @@ func TestRunCommand_Flags(t *testing.T) {
222221

223222
runPkgMock := new(RunPkgMock)
224223
runFunc = runPkgMock.Run
225-
runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
224+
runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(nil)
226225

227226
cmd := NewRunCommand(clients)
228227
testutil.MockCmdIO(clients.IO, cmd)
@@ -234,7 +233,7 @@ func TestRunCommand_Flags(t *testing.T) {
234233
// Check args passed into the run function
235234
if tc.expectedErr == nil {
236235
assert.NoError(t, err)
237-
runPkgMock.AssertCalled(t, "Run", mock.Anything, mock.Anything, mock.Anything,
236+
runPkgMock.AssertCalled(t, "Run", mock.Anything, mock.Anything,
238237
tc.expectedRunArgs,
239238
)
240239
} else {
@@ -256,7 +255,7 @@ func TestRunCommand_Help(t *testing.T) {
256255

257256
runPkgMock := new(RunPkgMock)
258257
runFunc = runPkgMock.Run
259-
runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
258+
runPkgMock.On("Run", mock.Anything, mock.Anything, mock.Anything).Return(nil)
260259

261260
err := cmd.ExecuteContext(ctx)
262261
assert.NoError(t, err)

internal/pkg/platform/localserver.go

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ import (
3333
"github.com/slackapi/slack-cli/internal/goutils"
3434
"github.com/slackapi/slack-cli/internal/hooks"
3535
"github.com/slackapi/slack-cli/internal/iostreams"
36-
"github.com/slackapi/slack-cli/internal/logger"
3736
"github.com/slackapi/slack-cli/internal/pkg/apps"
3837
"github.com/slackapi/slack-cli/internal/shared"
3938
"github.com/slackapi/slack-cli/internal/shared/types"
4039
"github.com/slackapi/slack-cli/internal/slackerror"
40+
"github.com/slackapi/slack-cli/internal/slacktrace"
41+
"github.com/slackapi/slack-cli/internal/style"
4142
)
4243

4344
// for lazy testing
@@ -78,7 +79,6 @@ type LocalHostedContext struct {
7879
// response management to the script hook
7980
type LocalServer struct {
8081
clients *shared.ClientFactory
81-
log *logger.Logger
8282
token string
8383
localHostedContext LocalHostedContext
8484
cliConfig hooks.SDKCLIConfig
@@ -143,7 +143,8 @@ func (r *LocalServer) Start(ctx context.Context) error {
143143

144144
// Listen waits for incoming events over a socket connection and invokes the deno-sdk-powered app with each payload. Responds to each event in a way that mimics the behaviour of a hosted app.
145145
func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done chan<- bool) {
146-
r.log.Info("on_cloud_run_connection_connected")
146+
r.clients.IO.PrintTrace(ctx, slacktrace.PlatformRunReady)
147+
r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary("Connected, awaiting events"))
147148

148149
// Listen for socket messages
149150
for {
@@ -169,8 +170,7 @@ func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done cha
169170
}
170171
return
171172
}
172-
r.log.Data["cloud_run_connection_message"] = string(messageBytes)
173-
r.log.Debug("on_cloud_run_connection_message")
173+
r.clients.IO.PrintDebug(ctx, "received: %s", string(messageBytes))
174174

175175
var msg Message
176176
err = json.Unmarshal(messageBytes, &msg)
@@ -222,13 +222,10 @@ func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done cha
222222

223223
if err != nil {
224224
// Log the error but do not return because the user may be able to recover inside their app code
225-
r.log.Data["cloud_run_connection_command_error"] = fmt.Sprintf("%s\n%s", err, out)
226-
r.log.Warn("on_cloud_run_connection_command_error")
225+
r.clients.IO.PrintError(ctx, "Error: %s\n%s", err, out)
227226
break
228227
}
229228

230-
r.log.Info("on_cloud_run_connection_command_output")
231-
232229
linkResponse = &LinkResponse{
233230
EnvelopeID: msg.EnvelopeID,
234231
Payload: json.RawMessage(out),
@@ -407,23 +404,19 @@ func (r *LocalServer) WatchManifest(ctx context.Context, auth types.SlackAuth, a
407404
return
408405
case event := <-w.Event:
409406
if isRemoteManifest {
410-
r.log.Data["cloud_run_watch_manifest_change_skipped"] = event.Path
411-
r.log.Info("on_cloud_run_watch_manifest_change_skipped_remote")
407+
r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("Manifest change detected: %s, skipped reinstalling app because manifest.source=remote", event.Path)))
412408
} else {
413-
r.log.Data["cloud_run_watch_manifest_change"] = event.Path
414-
r.log.Info("on_cloud_run_watch_manifest_change")
409+
r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("Manifest change detected: %s, reinstalling app...", event.Path)))
415410

416411
// Reinstall the app when manifest changes
417412
if _, _, _, err := apps.InstallLocalApp(ctx, r.clients, "", auth, app); err != nil {
418-
r.log.Data["cloud_run_watch_error"] = err.Error()
419-
r.log.Warn("on_cloud_run_watch_error")
413+
r.clients.IO.PrintError(ctx, "Error: %s", err)
420414
} else {
421-
r.log.Info("on_cloud_run_watch_manifest_change_reinstalled")
415+
r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary("App successfully reinstalled"))
422416
}
423417
}
424418
case err := <-w.Error:
425-
r.log.Data["cloud_run_watch_error"] = err.Error()
426-
r.log.Warn("on_cloud_run_watch_error")
419+
r.clients.IO.PrintError(ctx, "Error: %s", err)
427420
case <-w.Closed:
428421
return
429422
}
@@ -488,8 +481,7 @@ func (r *LocalServer) WatchApp(ctx context.Context) error {
488481
r.clients.IO.PrintDebug(ctx, "App file watcher context canceled, returning.")
489482
return
490483
case event := <-w.Event:
491-
r.log.Data["cloud_run_watch_app_change"] = event.Path
492-
r.log.Info("on_cloud_run_watch_app_change")
484+
r.clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf("App change detected: %s, restarting server...", event.Path)))
493485

494486
// Stop the previous process before starting a new one
495487
r.stopDelegateProcess(ctx)
@@ -503,8 +495,7 @@ func (r *LocalServer) WatchApp(ctx context.Context) error {
503495
}
504496
}()
505497
case err := <-w.Error:
506-
r.log.Data["cloud_run_watch_error"] = err.Error()
507-
r.log.Warn("on_cloud_run_watch_error")
498+
r.clients.IO.PrintError(ctx, "Error: %s", err)
508499
case <-w.Closed:
509500
return
510501
}

internal/pkg/platform/localserver_test.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/gorilla/websocket"
2828
"github.com/slackapi/slack-cli/internal/api"
2929
"github.com/slackapi/slack-cli/internal/hooks"
30-
"github.com/slackapi/slack-cli/internal/logger"
3130
"github.com/slackapi/slack-cli/internal/shared"
3231
"github.com/slackapi/slack-cli/internal/slackcontext"
3332
"github.com/slackapi/slack-cli/internal/slackerror"
@@ -140,12 +139,8 @@ func Test_LocalServer_Start(t *testing.T) {
140139
AppID: "A12345",
141140
TeamID: "justiceleague",
142141
}
143-
log := logger.Logger{
144-
Data: map[string]interface{}{},
145-
}
146142
server := LocalServer{
147143
clients,
148-
&log,
149144
"ABC123",
150145
localContext,
151146
clients.SDKConfig,
@@ -353,12 +348,8 @@ func Test_LocalServer_Listen(t *testing.T) {
353348
AppID: "A12345",
354349
TeamID: "justiceleague",
355350
}
356-
log := logger.Logger{
357-
Data: map[string]interface{}{},
358-
}
359351
server := LocalServer{
360352
clients,
361-
&log,
362353
"ABC123",
363354
localContext,
364355
clients.SDKConfig,

internal/pkg/platform/run.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ import (
2323
"github.com/slackapi/slack-cli/cmd/feedback"
2424
"github.com/slackapi/slack-cli/cmd/triggers"
2525
"github.com/slackapi/slack-cli/internal/config"
26-
"github.com/slackapi/slack-cli/internal/logger"
2726
"github.com/slackapi/slack-cli/internal/pkg/apps"
2827
"github.com/slackapi/slack-cli/internal/shared"
2928
"github.com/slackapi/slack-cli/internal/shared/types"
3029
"github.com/slackapi/slack-cli/internal/slackerror"
3130
"github.com/slackapi/slack-cli/internal/slacktrace"
31+
"github.com/slackapi/slack-cli/internal/style"
3232
)
3333

3434
// RunArgs are the arguments passed into the Run function
@@ -43,7 +43,7 @@ type RunArgs struct {
4343
}
4444

4545
// Run locally runs your app.
46-
func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, runArgs RunArgs) (*logger.LogEvent, types.InstallState, error) {
46+
func Run(ctx context.Context, clients *shared.ClientFactory, runArgs RunArgs) (types.InstallState, error) {
4747
span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.create")
4848
defer span.Finish()
4949

@@ -54,9 +54,9 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
5454
authSession, err := clients.API().ValidateSession(ctx, runArgs.Auth.Token)
5555
if err != nil {
5656
err = slackerror.Wrap(err, "No auth session found")
57-
return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
57+
return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
5858
}
59-
log.Data["teamName"] = *authSession.TeamName
59+
teamName := *authSession.TeamName
6060

6161
if authSession.UserID != nil {
6262
ctx = config.SetContextUserID(ctx, *authSession.UserID)
@@ -73,17 +73,17 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
7373
// For CLI Run command execute successfully
7474
if !clients.SDKConfig.Hooks.Start.IsAvailable() {
7575
var err = slackerror.New(slackerror.ErrSDKHookNotFound).WithMessage("The `start` script was not found")
76-
return nil, "", err
76+
return "", err
7777
}
7878

7979
// Update local install
8080
installedApp, localInstallResult, installState, err := apps.InstallLocalApp(ctx, clients, runArgs.OrgGrantWorkspaceID, runArgs.Auth, runArgs.App)
8181
if err != nil {
82-
return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
82+
return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
8383
}
8484

8585
if installState == types.InstallRequestPending || installState == types.InstallRequestCancelled || installState == types.InstallRequestNotSent {
86-
return log.SuccessEvent(), types.InstallSuccess, nil
86+
return types.InstallSuccess, nil
8787
}
8888

8989
if runArgs.ShowTriggers {
@@ -93,17 +93,17 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
9393
if strings.Contains(err.Error(), "workflow_not_found") {
9494
listErr := triggers.ListWorkflows(ctx, clients, runArgs.App, runArgs.Auth)
9595
if listErr != nil {
96-
return nil, "", slackerror.Wrap(listErr, "Error listing workflows").WithRootCause(err)
96+
return "", slackerror.Wrap(listErr, "Error listing workflows").WithRootCause(err)
9797
}
9898
}
99-
return nil, "", err
99+
return "", err
100100
}
101101
}
102102

103103
// Gather environment variables from an environment file
104104
variables, err := clients.Config.GetDotEnvFileVariables()
105105
if err != nil {
106-
return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun).
106+
return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun).
107107
WithMessage("Failed to read the local .env file")
108108
}
109109

@@ -123,7 +123,6 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
123123

124124
var server = LocalServer{
125125
clients: clients,
126-
log: log,
127126
token: localInstallResult.APIAccessTokens.AppLevel,
128127
localHostedContext: localHostedContext,
129128
cliConfig: cliConfig,
@@ -141,7 +140,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
141140
<-ctx.Done()
142141
clients.IO.PrintDebug(ctx, "Interrupt signal received in Run command, cleaning up and shutting down")
143142
if cleanup {
144-
deleteAppOnTerminate(ctx, clients, runArgs.Auth, installedApp, log)
143+
deleteAppOnTerminate(ctx, clients, runArgs.Auth, installedApp, teamName)
145144
}
146145
feedback.ShowFeedbackMessageOnTerminate(ctx, clients)
147146
// Notify Slack backend we are closing connection; this should trigger an echoing close message from Slack
@@ -196,21 +195,22 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger,
196195
if err := <-errChan; err != nil {
197196
switch slackerror.ToSlackError(err).Code {
198197
case slackerror.ErrLocalAppRunCleanExit:
199-
return log.SuccessEvent(), types.InstallSuccess, nil
198+
return types.InstallSuccess, nil
200199
case slackerror.ErrSDKHookInvocationFailed:
201-
return nil, "", err
200+
return "", err
202201
}
203-
return nil, "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
202+
return "", slackerror.Wrap(err, slackerror.ErrLocalAppRun)
204203
}
205-
return log.SuccessEvent(), types.InstallSuccess, nil
204+
return types.InstallSuccess, nil
206205
}
207206

208-
func deleteAppOnTerminate(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, log *logger.Logger) {
207+
func deleteAppOnTerminate(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, app types.App, teamName string) {
209208
clients.IO.PrintDebug(ctx, "Removing the local version of this app from the workspace")
210209
_, _, err := apps.Delete(ctx, clients, app.TeamDomain, app, auth)
211210
if err != nil {
212-
log.Data["on_cleanup_app_install_error"] = err.Error()
213-
log.Info("on_cleanup_app_install_failed")
211+
clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf(`Cleaning up local app install for "%s" failed.`, teamName)))
212+
clients.IO.PrintWarning(ctx, "Local app cleanup failed: %s", err)
213+
} else {
214+
clients.IO.PrintInfo(ctx, false, "%s", style.Secondary(fmt.Sprintf(`Cleaned up local app install for "%s".`, teamName)))
214215
}
215-
log.Info("on_cleanup_app_install_done")
216216
}

0 commit comments

Comments
 (0)