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

AMQP Listener Doesn't Work With dot values in the pin (e.g. role:twitter,version:v1.0,cmd:tweets,method:GET) #66

Closed
ericnograles opened this issue Oct 29, 2016 · 2 comments
Assignees
Labels

Comments

@ericnograles
Copy link
Contributor

Hi there!

First off, fantastic work on Seneca and its ecosystem. It is a revelation, especially after having "rolled our own" microservice conventions and knowing how difficult it is to land on a proper pattern. The Seneca scheme is quite elegant.

Issue Overview

I think I ran into a weird edge case, or I missed something in the documentation. In short, it appears that Seneca AMQP services don't seem to understand a pin if the value has dots in it.

Sample Use Case

For example, let's say I have a client that fires off role:twitter,version:v1.0,cmd:tweets,method:GET to an AMQP listener. Even if the receiving service specifies "any" for the version, it still won't understand the v1.0 bit of the pin.

However, firing off role:twitter,version:v1,cmd:tweets,method:GET will actually work.

I haven't had a chance to dive into the source code of seneca-amqp-transport yet to pinpoint how things might work (will do that later today), but I wanted to fire up this issue to see if there was a quick answer.

Code for the AMQP listener below. Thanks for your help, everyone, and keep up the great work!

AMQP Listener Code

var fs = require('fs');

/**
 * A sample AMQP microservice for service bus communication via RabbitMQ with the seneca-service-api
 * @param options
 */
function twitter(options) {
  var log;

  this.add('init:twitter', init);
  this.add('role:twitter,version:*,cmd:*,method:*', noMatch);
  this.add('role:twitter,version:*,cmd:tweets,method:GET', tweets);

  function init(msg, respond) {
    // Note, all the code below is optional
    // log to a custom file
    fs.open(options.logfile, 'a', function (err, fd) {

      // cannot open for writing, so fail
      // this error is fatal to Seneca
      if (err) return respond(err);

      log = makeLog(fd);
      respond();
    });
  }

  function makeLog(fd) {
    // TODO: Tie this into something like Winston
    return function (entry) {
      fs.write(fd, '\n' + new Date().toISOString() + ' ' + entry, null, 'utf8', function (err) {
        if (err) return console.log(err);

        // ensure log entry is flushed
        fs.fsync(fd, function (err) {
          if (err) return console.log(err);
        })
      });
    }
  }

  function noMatch(payload, respond) {
    respond(null, {
      statusCode: 404,
      original: payload,
      error: {
        message: 'Invalid service command'
      }
    });
  }

  function tweets(payload, respond) {
    // TODO: Your service begins here
    log('RECEIVED: ' + payload);
    respond(null, {status: 'success', tweets: []});
  }
}

// Define queues
require('seneca')()
  .use(twitter, { logfile: './twitter.log'})
  .use('seneca-amqp-transport')
  .listen({
    type: 'amqp',
    pin: 'role:twitter,version:*,cmd:*,method:*',
    url: process.env.AMQP_URL || 'amqp://127.0.0.1'
  });
@nfantone
Copy link
Collaborator

Hi, @ericnograles. Good catch there. It looks like a bug related to topic parsing. My guess is that some function on amqp-util.js is misinterpreting your pin. Unfortunately, I'll be out for the weekend so I won't be able to dig into this until next week.

Keep me posted if you find anything else.

@nfantone nfantone added the bug label Oct 29, 2016
@nfantone nfantone self-assigned this Oct 29, 2016
@ericnograles
Copy link
Contributor Author

ericnograles commented Oct 29, 2016

Thanks, @nfantone! I believe I found the root cause in amqp-util.js. Line 120 in the resolveTopic function.

image

The join with a simple period is where I believe it goes screwy. I'm guessing we're going to need to find a way to "escape" dots in pin values somehow. I'll try to work out a solution and PR back for review.

Come to think of it, the listener might also have an issue if we're defining a service listening on a pin with periods in the value. I'll try to shake out those use cases as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants