Skip to content
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.

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

Slack: Retry to auth if some scopes are missing #17640

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

mathieudutour
Copy link
Member

Description

Screencast

Checklist

@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: slack Issues related to the slack extension labels Mar 7, 2025
@raycastbot
Copy link
Collaborator

Thank you for your contribution! 🎉

🔔 @momme-rtf @Elliot67 @jfkisafk @thomaslombart @RobErskine you might want to have a look.

You can use this guide to learn how to check out the Pull Request locally in order to test it.

Due to our current reduced availability, the initial review may take up to 10-15 business days

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR adds a retry mechanism to handle missing Slack scopes by clearing tokens and re-authenticating when needed in the status setting functionality.

  • The global retried flag in /extensions/slack/src/tools/set-status.ts could cause race conditions if multiple status updates happen simultaneously - consider making it request-scoped
  • The error handling in /extensions/slack/src/tools/set-status.ts could be simplified using showFailureToast from @raycast/utils instead of throwing errors
  • The launchCommand in /extensions/slack/src/tools/set-status.ts should be wrapped in a try-catch block for better error handling

💡 (4/5) You can add custom instructions or style guidelines for the bot here!

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -20,6 +20,8 @@ type Input = {
duration?: number;
};

let retried = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Global retried flag could cause issues if multiple status updates are attempted in the same session. Consider moving this into the function scope.

Suggested change
let retried = false;
async function setStatus(input: Input) {
let retried = false;

Copy link
Member

@thomaspaulmann thomaspaulmann left a comment

Choose a reason for hiding this comment

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

Looks good and hopefully resolves the issue. I wonder if we should move that logic down into the utils. So other extensions would benefit of it as well.

@mathieudutour
Copy link
Member Author

res.error?.includes("missing_scope") is pretty specific to the Slack API - each API returns different stuff so hard to generalize

@mathieudutour mathieudutour merged commit 3fe47b9 into main Mar 8, 2025
8 of 11 checks passed
@mathieudutour mathieudutour deleted the f/retry-missing-scope branch March 8, 2025 13:57
Copy link
Contributor

github-actions bot commented Mar 8, 2025

Published to the Raycast Store:
https://raycast.com/mommertf/slack

@raycastbot
Copy link
Collaborator

🎉 🎉 🎉

We've rewarded your Raycast account with some credits. You will soon be able to exchange them for some swag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension fix / improvement Label for PRs with extension's fix improvements extension: slack Issues related to the slack extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants