Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

pubsub.subscribe option parameter not optional in 0.27 #1129

Closed
kevinsimper opened this issue Dec 5, 2017 · 4 comments
Closed

pubsub.subscribe option parameter not optional in 0.27 #1129

kevinsimper opened this issue Dec 5, 2017 · 4 comments
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@kevinsimper
Copy link

According to the docs, option is optional, but it is required in 0.27 as it will try to call the callback as the third parameter.
https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/PUBSUB.md#pubsubsubscribe

If only two parameters were given, both callback and handler would be undefined

callback = handler

@daviddias
Copy link
Member

@kevinsimper Wanna submit a test that shows that incorrect behavior?

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up help wanted Seeking public contribution on this issue labels Dec 7, 2017
@daviddias daviddias added status/ready Ready to be worked exp/novice Someone with a little familiarity can pick up labels Jan 25, 2018
@achingbrain
Copy link
Member

If two arguments are passed, callback and handler would indeed be undefined. The block in question reassigns options to handler if options is a function and then assigns handler to callback (to cover the case where you've passed three arguments). Later on we guard on callback and return a promise if it's falsey.

So if there are two args it should be ok as it'll just result in a promise being returned from the function call, which is what happens in this test case.

@kevinsimper have you observed any unexpected behaviour?

@kevinsimper
Copy link
Author

kevinsimper commented Feb 16, 2018

@diasdavid Sorry for the missing response!

It has been fixed in the newest version of ipfs 0.27.7 and I just tested again and got the error still in 0.27.0, so good 👍

https://jsbin.com/zuvuxijuma/edit?html,js,output

var id = 'ipfs-repo-' + String(Math.random())
console.log({id})
const node = new Ipfs({
  repo: id,
  EXPERIMENTAL: {
    pubsub: true
  },
  config: {
    Addresses: {
      Swarm: [
        '/dns4/ws-star.discovery.libp2p.io/tcp/443/wss/p2p-websocket-star'
      ]
    }
  }
})

node.on('ready', function() {
  // Your node is now ready to use \o/
  console.log('ready')
  // stopping a node

  let topic = 'fruit-of-the-daykevin'

  let receiveMsg = function(msg) {
    console.log(msg.data.toString(), Date.now())
  }

  node.pubsub.subscribe(topic, receiveMsg)
  setInterval(function() {
    node.pubsub.peers(topic, (err, peerIds) => {
      if (err) {
        throw err
      }
      console.log(peerIds)
    })
  }, 1000)
  setInterval(function() {
    node.pubsub.publish(topic, new node.types.Buffer('apple ' + id + ' ' + Date.now()), function(
      e
    ) {
      console.log({ e })
    })
  }, 4000)
})

This will fail:

  <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/index.js"></script>

with this error:

pubsub.js:27 Uncaught TypeError: callback is not a function
    at setImmediate (pubsub.js:27)
    at setImmediate.js:27
    at run (setImmediate.js:40)
    at runIfPresent (setImmediate.js:69)
    at onGlobalMessage (setImmediate.js:109)

However works fine now with:

<script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/index.js"></script>

@daviddias
Copy link
Member

👍 great, thanks for letting us know @kevinsimper

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants