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

fix: issue with publish ignoring 'drain' event. #163

Merged
merged 1 commit into from
Aug 21, 2021

Conversation

richardklafter
Copy link
Contributor

This PR addresses issue #129.

There didn't seem to be a good way to test it within the testing framework in the repository so I wrote a small repo script.

var amqp = require('./lib/index');
async function main() {
  var connection = amqp.connect(['amqp://guest:guest@localhost']);
  var channelWrapper = connection.createChannel({
    setup: function (channel) {
      return channel.assertQueue('rxQueueName', { durable: true });
    }
  });


  //this way works fine
  console.log('starting serial queueing...')
  for (let i = 0; i < 200000; i++) {
    await channelWrapper.sendToQueue('rxQueueName', Buffer.from('serial'+i))
  }
  console.log(`finished serial queueing`)

  //this way fails because is ignores drain
  console.log('starting parallel queueing')
  const promises = []
  for (let i = 0; i < 200000; i++) {
    promises.push(channelWrapper.sendToQueue('rxQueueName', Buffer.from('parallel'+i)))
  }
  setTimeout(()=>{
    console.error('It has been 10 mins. Good chance your promises are never coming back :(')
  }, 1000*60*10)
  await Promise.all(promises)
  console.log(`finished parallel queueing`)
}
main().then(()=>console.log('Done testing!')).catch(console.error).then(()=>process.exit())

Output before this PR:

Successfully compiled 4 files with Babel (520ms).
Done in 0.65s.
starting serial queueing...
finished serial queueing
starting parallel queueing
It has been 10 mins. Good chance your promises are never coming back :(

Output after this PR:

Successfully compiled 4 files with Babel (416ms).
Done in 0.52s.
starting serial queueing...
finished serial queueing
starting parallel queueing
finished parallel queueing
Done testing!

@jwalton
Copy link
Owner

jwalton commented Aug 21, 2021

Thanks so much for this! One small request - can you change your commit message to something like:

fix: fixed issue with publish ignoring 'drain' event

fix #129

? This uses semantic-release, so the commit message has to follow that format for the release notes to be generated correctly. I can fix it with a squash merge, but then you won't show up as a "contributor" in github, and some people are touchy about that. ;)

Good chance your promises are never coming back

Hahaha

@richardklafter
Copy link
Contributor Author

Updated commit message. If it is still wrong feel free to squash merge. Thanks!

@jwalton jwalton merged commit 8cb10a5 into jwalton:master Aug 21, 2021
@jwalton
Copy link
Owner

jwalton commented Aug 21, 2021

Thanks again!

@jwalton
Copy link
Owner

jwalton commented Aug 21, 2021

Unit tests are failing (Why didn't github actions run on this PR? -_-) I'll see if I can have a look tonight.

@jwalton
Copy link
Owner

jwalton commented Aug 21, 2021

🎉 This PR is included in version 3.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

jwalton added a commit that referenced this pull request Aug 22, 2021
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.

2 participants