Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented May 29, 2025

Changelog

Outputs from erroring hook scripts - such found when gathering an odd app manifest - are no longer hidden in verbose outputs and are instead shown with invocation errors.

Summary

This PR includes the SDK hook script error in CLI error outputs to improve the debug experience, as @WilliamBergamin noted 🙏 ✨

Preview

Before changes

before.mov

After changes

after.mov

Reviewers

The following commands can be used to check these erroring outputs:

$ slack create asdf -t slack-samples/bolt-js-starter-template
$ cd asdf
$ slack manifest  # No errors we hope
$ echo "{" >> manifest.json"
$ slack manifest  # Errors with detail
$ vim .slack/hooks.json
{
  "hooks": {
    "get-hooks": "npx -q --no-install -p @slack/cli-hooks slack-cli-get-hooks"
  },
  "config": {
    "protocol-version": ["default"]
  }
}
$ slack manifest  # Default protocol also has details

Notes

  • Surfacing errors parsing .slack/hooks.json is left for a followup PR, but these are shown in verbose outputs for now 🙏
  • Exit codes might not match what's returned in hook scripts and updates are left for a follow up PR also!

Requirements

@zimeg zimeg added this to the Next Release milestone May 29, 2025
@zimeg zimeg requested a review from WilliamBergamin May 29, 2025 17:40
@zimeg zimeg self-assigned this May 29, 2025
@zimeg zimeg requested a review from a team as a code owner May 29, 2025 17:40
@zimeg zimeg added enhancement M-T: A feature request for new functionality changelog Use on updates to be included in the release notes semver:patch Use on pull requests to describe the release version increment labels May 29, 2025
@codecov
Copy link

codecov bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.49%. Comparing base (d294a85) to head (e6d159e).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   63.47%   63.49%   +0.02%     
==========================================
  Files         212      212              
  Lines       22331    22341      +10     
==========================================
+ Hits        14174    14185      +11     
+ Misses       7067     7065       -2     
- Partials     1090     1091       +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.

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

Nice 💯

@mwbrooks mwbrooks modified the milestones: v3.3.0, Next Release May 30, 2025
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

📝 A recent change after additional testing of erroneous cases for the wonderful reviewers more!

Comment on lines +75 to +79
// Include stderr outputs in error details if these aren't streamed
details := slackerror.ErrorDetails{}
if opts.Stderr == nil {
details = append(details, slackerror.ErrorDetail{Message: strings.TrimSpace(bufferr.String())})
}
Copy link
Member Author

@zimeg zimeg Jun 2, 2025

Choose a reason for hiding this comment

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

📣 This guards against duplicating outputs that are streamed for certain hooks, such as local run for ROSI apps or a deploy hook script.

Before changes:

before


After changes:

after

Copy link
Member

@mwbrooks mwbrooks left a comment

Choose a reason for hiding this comment

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

✅ Great improvement to debugging errors in SDK hooks! 🎉 And I'm surprised at how small the change was to do it.

@zimeg
Copy link
Member Author

zimeg commented Jun 5, 2025

@WilliamBergamin @mwbrooks Thank y'all both for suggesting these changes and iterations needed for improved error handling with hooks - this is much more clear than parsing debug logs IMO and I hope makes finding fixes quick.

I'll merge this now so we can test this in development! 👾 ✨

@zimeg zimeg merged commit 662186b into main Jun 5, 2025
6 checks passed
@zimeg zimeg deleted the zimeg-feat-hook-error-details branch June 5, 2025 06:53
@mwbrooks mwbrooks added semver:minor Use on pull requests to describe the release version increment and removed semver:patch Use on pull requests to describe the release version increment labels Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog Use on updates to be included in the release notes enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants