Skip to content

fix: serialize getting message types to avoid slack rate limits #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
korotovsky merged 1 commit into from
Sep 22, 2025
Merged

fix: serialize getting message types to avoid slack rate limits #97

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions pkg/provider/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,13 +549,9 @@ func (ap *ApiProvider) GetSlackConnect(ctx context.Context) ([]slack.User, error
return res, nil
}

func (ap *ApiProvider) GetChannels(ctx context.Context, channelTypes []string) []Channel {
if len(channelTypes) == 0 {
channelTypes = AllChanTypes
}

func (ap *ApiProvider) GetChannelsType(ctx context.Context, channelType string) []Channel {
params := &slack.GetConversationsParameters{
Types: AllChanTypes,
Types: []string{channelType},
Limit: 999,
ExcludeArchived: true,
}
Expand All @@ -575,6 +571,10 @@ func (ap *ApiProvider) GetChannels(ctx context.Context, channelTypes []string) [
}

channels, nextcur, err = ap.client.GetConversationsContext(ctx, params)
ap.logger.Debug("Fetched channels for ",
zap.String("channelType", channelType),
zap.Int("count", len(channels)),
)
if err != nil {
ap.logger.Error("Failed to fetch channels", zap.Error(err))
break
Expand All @@ -599,17 +599,30 @@ func (ap *ApiProvider) GetChannels(ctx context.Context, channelTypes []string) [
chans = append(chans, ch)
}

for _, ch := range chans {
ap.channels[ch.ID] = ch
ap.channelsInv[ch.Name] = ch.ID
}

if nextcur == "" {
break
}

params.Cursor = nextcur
}
return chans
}

func (ap *ApiProvider) GetChannels(ctx context.Context, channelTypes []string) []Channel {
if len(channelTypes) == 0 {
channelTypes = AllChanTypes
}

var chans []Channel
for _, t := range AllChanTypes {
Copy link
Owner

@korotovsky korotovsky Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not this line be range channelTypes {?

Copy link
Contributor Author

@orinciog orinciog Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also asked myself, but I left it as it was: https://github.com/korotovsky/slack-mcp-server/blob/master/pkg/provider/api.go#L558

Copy link
Owner

@korotovsky korotovsky Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I can explain how it happened and you might improve it in this PR? Basically background goroutine must fetch all channels of all types no matter what to provide consistent cache, before cache was introduced this function getChannels() have been used directly from tool handler with parameter channel types provided from the user.

var typeChannels = ap.GetChannelsType(ctx, t)
chans = append(chans, typeChannels...)
}

for _, ch := range chans {
ap.channels[ch.ID] = ch
ap.channelsInv[ch.Name] = ch.ID
}

var res []Channel
for _, t := range channelTypes {
Expand Down