fix: 'run' exiting on disconnected api errors when streaming activity logs by zimeg · Pull Request #132 · slackapi/slack-cli

zimeg · 2025-06-19T06:12:34Z

Changelog

We fixed a bug where the run command exited with an error if the activity logs failed to stream to now attempt retries for those missed logs.

Summary

This PR avoids exiting on an error when streaming the activity logs for the run command to workaround or fix #128! 👾

Preview

The following video shows a disconnected and reconnected internet connection and a healing socket connection:

0:07: The app has started and connected
0:19: The internet connection is turned off
0:52: A new web socket attempts to connect
1:15: Activity logs begin to fail retries - New
1:37: The internet connection is turned on
1:40: A single socket mode connection is created with streaming logs

disconnect.mov

Notes

This change doesn't fix the cause of persisting processes with a cancelled context or exit without error, but we instead avoid both to require interrupts to or process exits from the app!
Following the note above, the "Closing due to inactivity. Au revoir!" might need to include an error instead. Perhaps we save this or canceling the context on exits for a follow up PR? 🤖

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

codecov · 2025-06-19T06:14:39Z

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.57%. Comparing base (4d342d7) to head (583bea7).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/api/activity.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
+ Coverage   63.48%   63.57%   +0.09%     
==========================================
  Files         212      212              
  Lines       22345    22348       +3     
==========================================
+ Hits        14185    14208      +23     
+ Misses       7078     7057      -21     
- Partials     1082     1083       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zimeg

📝 A few of the thoughts happening with some of these changes for the most kind reviewers!

zimeg · 2025-06-19T06:20:18Z

internal/api/activity.go

 	b, err := c.get(ctx, url, token, "")
 	if err != nil {
-		return ActivityResult{}, errHTTPRequestFailed.WithRootCause(err)
+		return ActivityResult{}, slackerror.New(slackerror.ErrHTTPRequestFailed).WithRootCause(err)


📣 We create a new error here to avoid appending different repeated root causes to this same error:

HTTP request failed (http_request_failed) Get "https://slack.com/api/apps.activities.list?app_id=A0923PDU39B&limit=100&min_log_level=info&min_date_created=1750312064888018": dial tcp: lookup slack.com: no such host Get "https://slack.com/api/apps.activities.list?app_id=A0923PDU39B&limit=100&min_log_level=info&min_date_created=1750312064888018": dial tcp: lookup slack.com: no such host Get "https://slack.com/api/apps.activities.list?app_id=A0923PDU39B&limit=100&min_log_level=info&min_date_created=1750312064888018": dial tcp: lookup slack.com: no such host Get "https://slack.com/api/apps.activities.list?app_id=A0923PDU39B&limit=100&min_log_level=info&min_date_created=1750312064888018": dial tcp: lookup slack.com: no such host

zimeg · 2025-06-19T06:21:25Z

internal/pkg/platform/activity.go

+		if err != nil {
+			return err
+		}
 		return nil


🔍 Small preference to make the nil return more clear instead of returning err in both cases, even if it too is nil.

Thanks, I prefer this clarify as well!

zimeg · 2025-06-19T06:38:07Z

internal/pkg/platform/activity_test.go

 				cm.API.AssertNumberOfCalls(t, "Activity", 1)
 			},
 		},
+		"should return nil if TailArg is set and activity request fails while polling": {


🧪 The added nil values for ExpectedError above are included to make these cases more explicit instead of preferring default behavior.

For this test I think it's useful, but I'm of course open to reverting this!

No need to revert, I like this as well!

mwbrooks

✅ Nice improvement @zimeg!

🧪 During testing, I noticed that a few times I'd receive a connection reset by peer error when re-connecting the wifi network. However, it doesn't always happen.

🎥 Below is a video for anyone who wants to see the original error that this PR fixes:

2025-06-23-run-disconnect.mov

mwbrooks · 2025-06-20T23:49:15Z

internal/pkg/platform/activity.go

+		if err != nil {
+			return err
+		}
 		return nil


Thanks, I prefer this clarify as well!

mwbrooks · 2025-06-20T23:51:41Z

internal/pkg/platform/activity_test.go

 				cm.API.AssertNumberOfCalls(t, "Activity", 1)
 			},
 		},
+		"should return nil if TailArg is set and activity request fails while polling": {


No need to revert, I like this as well!

zimeg · 2025-06-25T01:34:42Z

@mwbrooks Thanks so much for testing this! 🎁

I noticed that a few times I'd receive a connection reset by peer error when re-connecting the wifi network. However, it doesn't always happen.

Woah this might be happening for deno apps with the CLI-managed server in place of delegated connection? 👾

The retries of @slack/socket-mode have been persistent in my testing but we should explore that error soon:

slack-cli/internal/pkg/platform/localserver.go

Line 116 in 4d342d7

    
           r.clients.IO.PrintDebug(ctx, "LocalServer.Start closing websocket TCP connection")

The before video was also forgotten in these hacks - thank you for sharing this too 📸

I'll merge this now to find out if I can keep a connection ongoing these next multiple hours and will report back 🫡

zimeg · 2025-06-25T06:50:37Z

🔍 A CLI error has since appeared for me after running the app for a while and repeats every few seconds but without exiting which I think is expected:

[2025-06-24 18:39:00] Setting up a ticker to poll activity on a 3s interval
...
[INFO]  bolt-app ⚡️ Bolt app is running!
[2025-06-24 22:10:18] HTTP Request: GET https://slack.com/api/apps.activities.list?app_id=A093KR40YEL&limit=100&min_log_level=info&min_date_created=1750815539904263 HTTP/1.1
[2025-06-24 22:10:18] HTTP Request Body:
[2025-06-24 22:10:18] <no body>
[2025-06-24 22:10:18] HTTP Response Status: HTTP/1.1 200 OK
[2025-06-24 22:10:18] HTTP Response Body:
[2025-06-24 22:10:18] <no body>
[2025-06-24 22:10:18] The following error was returned by the apps.activities.list Slack API method
[2025-06-24 22:10:18]
[2025-06-24 22:10:18] 🚫 Your access token has expired (token_expired)
[2025-06-24 22:10:18]
[2025-06-24 22:10:18] 💡 Suggestion
[2025-06-24 22:10:18]    Use the command `lack login` to authenticate again
...
[2025-06-24 22:14:18] HTTP Request: GET https://slack.com/api/apps.activities.list?app_id=A093KR40YEL&limit=100&min_log_level=info&min_date_created=1750815539904263 HTTP/1.1
[2025-06-24 22:14:18] HTTP Request Body:
[2025-06-24 22:14:18] <no body>
[2025-06-24 22:14:18] HTTP Response Status: HTTP/1.1 200 OK
[2025-06-24 22:14:18] HTTP Response Body:
[2025-06-24 22:14:18] <no body>
[2025-06-24 22:14:18] The following error was returned by the apps.activities.list Slack API method
[2025-06-24 22:14:18]
[2025-06-24 22:14:18] 🚫 Your access token has expired (token_expired)
[2025-06-24 22:14:18]
[2025-06-24 22:14:18] 💡 Suggestion
[2025-06-24 22:14:18]    Use the command `lack login` to authenticate again
[WARN]  socket-mode:SlackWebSocket:1 A pong wasn't received from the server before the timeout of 5000ms!
[WARN]  socket-mode:SlackWebSocket:1 A ping wasn't received from the server before the timeout of 30000ms!
[DEBUG]  socket-mode:SlackWebSocket:1 Sending close frame (status=1000).
[DEBUG]  socket-mode:SlackWebSocket:1 Sending close frame (status=1000).
[DEBUG]  socket-mode:SlackWebSocket:1 WebSocket close frame received (code: 1006, reason: )
[DEBUG]  socket-mode:SlackWebSocket:1 Terminating WebSocket (close frame received).
[DEBUG]  socket-mode:SocketModeClient:0 Before trying to reconnect, this client will wait for 5000 milliseconds

I'm wondering if an expired token for apps.activities.list isn't being rotated or used in new requests? 👾

fix: avoid exiting on error when streaming activity logs

2f593d0

zimeg added this to the Next Release milestone Jun 19, 2025

zimeg requested review from WilliamBergamin and mwbrooks June 19, 2025 06:12

zimeg self-assigned this Jun 19, 2025

zimeg requested a review from a team as a code owner June 19, 2025 06:12

zimeg added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment labels Jun 19, 2025

zimeg mentioned this pull request Jun 19, 2025

bug: unexpected daemon process #128

Closed

3 tasks

zimeg added 2 commits June 18, 2025 23:24

test: mock an example error code when creating a new error

0d6e830

test: make the expected error states and test names match setup

77a3d7d

zimeg commented Jun 19, 2025

View reviewed changes

zimeg changed the title ~~fix: avoid exiting on error when streaming activity logs~~ fix: avoid exiting on disconnected api errors when streaming activity logs Jun 19, 2025

Merge branch 'main' into zimeg-fix-activity-tail-disconnect

583bea7

mwbrooks changed the title ~~fix: avoid exiting on disconnected api errors when streaming activity logs~~ fix: 'run' exiting on disconnected api errors when streaming activity logs Jun 23, 2025

mwbrooks approved these changes Jun 23, 2025

View reviewed changes

zimeg merged commit 0faf469 into main Jun 25, 2025
6 checks passed

zimeg deleted the zimeg-fix-activity-tail-disconnect branch June 25, 2025 01:35

Comments

Conversation

zimeg commented Jun 19, 2025

Summary

Preview

Notes

Requirements

Uh oh!

codecov bot commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

mwbrooks Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

mwbrooks Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

mwbrooks Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

mwbrooks Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg commented Jun 25, 2025

Uh oh!

Uh oh!

zimeg commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Jun 19, 2025 •

edited

Loading

zimeg commented Jun 25, 2025 •

edited

Loading