slackapi / slackapi/bolt-js

bug: bolt does not trigger handlers consistently when handler pattern contains stateful regex flags (`/y` or `/g`)

Open
#1,058 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

auto-triage-skip bug
Dominant language
TypeScript
Stars
2.9k
Forks
445
Avg merge
1d 3h
Merged PRs (30d)
28

Description

Description

Hi There,

Not really a Bolt-js issue, more an unexpected behavior and bad code from the net. You can close this issue when you want.

It took me quite some time to figure what was happening, so I'm posting it there if anyone run into the same kind of issue.

Problem:

  • calling a command /hello a first time works
  • calling the same command a second time ... timeout and /hello failed with the error "operation_timeout"
  • the server log only show one line, pointing to a ack() issue [ERROR] An incoming event was not acknowledged within 3 seconds. Ensure that the ack() argument is called in a listener.
  • after waiting a bit, I can run the command again, but only once
  • if I run another command, it works once, then timeout too
  • when it timeout, it doesnt even reach my listener or any part of my bolt code
  app.command(/(\/hello-dev|\/hello)/g, async ({ command, ack, respond }) => {
    await ack()
    await sayHello(command, respond)
  })

Solution:
After debugging all over bolt and expressReceiver, I figured that the culprit is around here

The command regexp is tested with return pattern.test(candidate); and any flag immediately become problematic.
My bad for copying a bad regexp from a bolt tutorial without noticing that.

So changing the regexp suddenly solved the problem (getting rid of flags like /g, /y)

app.command(/^\/(hello-dev|hello).*/, async ({ command, ack, respond }) => {
    await ack()
    await sayHello(command, respond)
})

Idea for improvement:

  • a warning message if not a single listener match, it would be way more helpful than a timeout
  • not showing a warning related to ack() if not a single listener match
  • a check on .lastIndex or .flags to check that regexp are not carrying a state over queries
What type of issue is this? (place an x in one of the [ ])
  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code related
  • testing related
  • discussion
Requirements (place an x in each of the [ ])
  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at src/middleware/builtin.ts around line 261, where the command regexp is tested with pattern.test(candidate), and reproduce the repeated /hello behavior using a handler pattern with /g or /y. Trace how a failed match prevents the listener from being reached and inspect the related middleware flow. Done means the repeated-command case is handled consistently and the relevant behavior is covered by tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.