Skip to content

[Due for payment 2025-08-20] Move App PR test failure announcements from #announce to #expensify-open-source #66645

Closed

Description

@arosiclair

Background

When an App PR is merged to main, we re-run checks (Process new code merged to main) to make sure they still pass. If they fail, we automatically

  1. Create an hourly Workflow Failure issue, auto-assign the person who merged the PR and mention the PR author
  2. Post the failure to #announce and tag @app_deployers

Per GH search, this has happened 57 times in the last 30 days.

Problems

  1. The slack announcement is noisy for the deployer team since broken checks do not require any special privileges or expertise from the team to fix.
  2. #announce is internal only so contributors are not notified and a deployer must manually enlist them to fix the issue

Solution

Move the slack announcement to #expensify-open-source to notify everyone about failures on main and tag only the engineer that merged the PR to investigate.

Activity

  1. self-assigned this
    on Jul 17, 2025
  2. changed the title [-]Notify everyone and tag the appropriate when checks start failing on main[/-] [+]Notify everyone and tag the appropriate engineer when checks start failing on main[/+] on Jul 17, 2025
  3. arosiclair commented on Jul 18, 2025

    @arosiclair
    ContributorAuthor

    Looks like the announcement is triggered here. It runs announceFailedWorkflowInSlack which always mentions app deployers.

    We could just set the CHANNEL to #expensify-open-source and add another MENTION param to switch who we're tagging. We can mention the github.actor, but that might not always match their slack handle 🤔.

  4. arosiclair commented on Jul 23, 2025

    @arosiclair
    ContributorAuthor

    Just realized we already have a GH action that automatically creates an hourly Workflow Failure issue and auto assigns it to the person who merged the PR - failureNotifier.yml.

    Here's an example: #66681

    So the slack announcement seems pretty redundant to me honestly. We could still share the issue in #expensify-open-source for visibility though.

  5. arosiclair commented on Jul 23, 2025

    @arosiclair
    ContributorAuthor

    Since failureNotifier already does what we want, we can just update that to also post the slack announcement. I updated the P/S with that context and I'll share it in #deployer.

  6. changed the title [-]Notify everyone and tag the appropriate engineer when checks start failing on main[/-] [+]Move App PR test failure announcements from #announce to #expensify-open-source[/+] on Jul 29, 2025
  7. arosiclair commented on Jul 29, 2025

    @arosiclair
    ContributorAuthor

    @AndrewGable it looks like you actually implemented this already here. Any idea why that doesn't seem to be working?

  8. AndrewGable commented on Jul 29, 2025

    @AndrewGable
    Contributor

    I would assume this logic is wrong, I remember looking into logs but didn't get far.

  9. arosiclair commented on Aug 7, 2025

    @arosiclair
    ContributorAuthor

    I would assume this logic is wrong, I remember looking into logs but didn't get far.

    Testing this locally with a test repo and it seems to be working correctly to me 😕

  10. AndrewGable commented on Aug 7, 2025

    @AndrewGable
    Contributor

    Can we add more logs to see what the shape is and/or if we are just not getting them to our webhook?

  11. arosiclair commented on Aug 7, 2025

    @arosiclair
    ContributorAuthor

    Ah okay I found an error in logs 967e56f15c67c938-SJC

    Failed to send Slack notification for preDeploy workflow failure Expensify\Libs\Error\SlackError - 5562DDC0-E7C8-47E2-8BD8-56B50FB8F4D3 ~~ githubActor: 'Julesssss' workflowRunUrl: 'https://github.com/Expensify/App/actions/runs/16653911753' exceptionJsonCode: '666' exceptionUserMessage: 'Slack error: channel_not_found' exceptionMessage: 'Slack error: channel_not_found' exceptionFile: '/git/releases/expensify.com/f7faba3/vendor/expensify/php-libs/src/Slack/Client.php' exceptionLine: '493' exceptionCode: '666'
    

    So the code is working, but #announce is not found for some reason

  12. 1 remaining item

  13. AndrewGable commented on Aug 7, 2025

    @AndrewGable
    Contributor

    Maybe we should try passing an array?

    $slackClient->announce([Slack_Client::CHANNEL_ANNOUNCE], $message);

  14. arosiclair commented on Aug 7, 2025

    @arosiclair
    ContributorAuthor

    Oh man I just realized "#announce" was spelled wrong 😄. I'm just going to update it to the #expensify-open-source channel

  15. AndrewGable commented on Aug 7, 2025

    @AndrewGable
    Contributor

    Oops! 🙈 Nice find!

  16. added
    ReviewingHas a PR in review
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Aug 7, 2025
  17. changed the title [-]Move App PR test failure announcements from #announce to #expensify-open-source[/-] [+][Due for payment 2025-08-20] Move App PR test failure announcements from #announce to #expensify-open-source[/+] on Aug 13, 2025
  18. melvin-bot commented on Aug 13, 2025

    @melvin-bot

    Reviewing label has been removed, please complete the "BugZero Checklist".

  19. melvin-bot commented on Aug 13, 2025

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.93-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2025-08-20. 🎊

  20. arosiclair commented on Aug 14, 2025

    @arosiclair
    ContributorAuthor

    All done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionImprovementItem broken or needs improvement.WeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions