Skip to content

feat: log the reason a request failed when errors are not fatal - #666

Merged
zimeg merged 4 commits into
slackapi:mainfrom
deepunyk:feat/log-failed-requests
Aug 11, 2026
Merged

feat: log the reason a request failed when errors are not fatal#666
zimeg merged 4 commits into
slackapi:mainfrom
deepunyk:feat/log-failed-requests

Conversation

@deepunyk

Copy link
Copy Markdown
Contributor

Summary

Closes #502.

The errors option defaults to false so a failed request does not fail the step. Nothing was written to the log either, so a workflow with a mistyped token or an invalid payload finished green with no output at all and no hint about what went wrong. The reason was only reachable by adding a follow-up step that echoed the response output.

This logs a warning with the reason instead, and mentions the errors input for workflows that do want the step to fail. The ok and response outputs are unchanged, and errors: true still fails the step with an error annotation rather than a warning.

Preview

An invalid token with the default options:

- name: Post a release announcement
  uses: slackapi/slack-github-action@v4.0.0
  with:
    method: chat.postMessage
    token: ${{ secrets.SLACK_BOT_TOKEN }}
    payload: |
      channel: "C0123456789"
      text: "*Starting a new release.*"
Warning: Failed to send the request: An API error occurred: invalid_auth. Set the "errors" input to "true" to fail this step for these errors.

Testing

Three tests are added to the existing send specifications, since this is where the errors option is handled:

  • the warning is logged with the API error from a failed method call
  • the warning is logged with the error from a failed webhook
  • no warning is logged when errors: true, which fails the step instead
$ npm run lint
$ npm run check
$ npm test  # 85 tests, 85 pass

Requirements

The step continues by default when a request fails, but nothing was
written to the log, so a misconfigured token or an invalid payload gave
no feedback at all. Warn with the reason instead, and point at the
"errors" input for workflows that want the step to fail.

Closes slackapi#502
@deepunyk
deepunyk requested review from a team as code owners August 11, 2026 15:44
@changeset-bot

changeset-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5781fa3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@slack/slack-github-action Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (e9c6c68) to head (5781fa3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #666   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files           8        8           
  Lines         731      732    +1     
=======================================
+ Hits          730      731    +1     
  Misses          1        1           

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zimeg

zimeg commented Aug 11, 2026

Copy link
Copy Markdown
Member

@deepunyk Thanks for sending this in! 🏁 I'm finding #552 still causes some tests to fail so will test a few more edge cases before finishing a review.

@zimeg zimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@deepunyk Thank you for bringing this issue to a PR! I made a few changes before we merge this, and am hoping to include a few others before it's released:

📚 In warning output I'm avoiding recommendations to change the value of "errors" because this might be intentionally set. Instead we can follow up to documentation improvements to highlight this more if needed!

Otherwise I think this is a nice set of focused changes! 🚀

Comment thread .changeset/warn-failed-requests.md Outdated
Comment thread .changeset/warn-failed-requests.md Outdated
Co-authored-by: Eden Zimbelman <zim@o526.net>
@zimeg
zimeg merged commit c28a16b into slackapi:main Aug 11, 2026
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errors during sending are not printed out

2 participants