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

Problem with star wildcards and NATS #517

Closed
levia7han opened this issue Apr 9, 2019 · 9 comments
Closed

Problem with star wildcards and NATS #517

levia7han opened this issue Apr 9, 2019 · 9 comments

Comments

@levia7han
Copy link

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • [x ] I am running the latest version
  • [x ] I checked the documentation and found no answer
  • [x ] I checked to make sure that this issue has not already been filed
  • [x ] I'm reporting the issue to the correct repository

Expected Behavior

Subscribing to ** event should capture all events emitted or broadcast from services over NATS transport

Current Behavior

** does not seem to catch anything

Failure Information

Events are not triggered for ** subscriptions when multiple services are dispatching events across NATS transportation

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. step 1
  2. step 2
  3. you get it...

Reproduce code snippet

const broker = new ServiceBroker({
    logger: console,
    transporter: "NATS"
});

broker.createService({
    name: "test",
    actions: {
        emit(ctx) {
            ctx.emit('this.is.an.event');
        }
    },
    events: {
      "**"(data, sender, eventName){
            this.logger.info("event triggered", eventName) 
      }
    }
});

Example ready to run

https://github.com/levia7han/moleculer-wildcard

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Moleculer version: 13.8
  • NodeJS version: 10.15.0
  • Operating System: windows

Failure Logs


Background

Attempting to write a service that logs all events broadcast or emitted within the system. I noticed that although unit tests were working, when we tried to get integration tests and real life testing to work we were not seeing the events we expected in the logs.

After digging into the code it seems like there is an issue with moleculer using * as a wildcard character where NATS expects > as a wildcard.

I have a pull request that adds the > wildcard along with tests that seems to fix this issue. It is located at https://github.com/levia7han/moleculer/tree/nats-wildcard

@icebob icebob added Priority: High Status: Need reproduce Issue waits for reproducing labels Apr 9, 2019
@icebob
Copy link
Member

icebob commented Apr 9, 2019

I can't reproduce it. Your code snippet is working properly:

[2019-04-09T19:10:15.985Z] INFO  bobcsi-pc-11044/BROKER: Moleculer v0.13.8 is starting...
[2019-04-09T19:10:15.988Z] INFO  bobcsi-pc-11044/BROKER: Node ID: bobcsi-pc-11044
[2019-04-09T19:10:15.989Z] INFO  bobcsi-pc-11044/BROKER: Namespace: <not defined>
[2019-04-09T19:10:15.990Z] INFO  bobcsi-pc-11044/REGISTRY: Strategy: RoundRobinStrategy
[2019-04-09T19:10:15.993Z] INFO  bobcsi-pc-11044/BROKER: Serializer: JSONSerializer
[2019-04-09T19:10:15.995Z] INFO  bobcsi-pc-11044/BROKER: Transporter: NatsTransporter
[2019-04-09T19:10:15.996Z] INFO  bobcsi-pc-11044/BROKER: Registered 10 internal middleware(s).
[2019-04-09T19:10:16.004Z] INFO  bobcsi-pc-11044/TRANSIT: Connecting to the transporter...
[2019-04-09T19:10:16.360Z] INFO  bobcsi-pc-11044/TRANSPORTER: NATS client is connected.
[2019-04-09T19:10:16.884Z] INFO  bobcsi-pc-11044/REGISTRY: '$node' service is registered.
[2019-04-09T19:10:16.885Z] INFO  bobcsi-pc-11044/REGISTRY: 'test' service is registered.
[2019-04-09T19:10:16.886Z] INFO  bobcsi-pc-11044/TEST: event triggered $services.changed
[2019-04-09T19:10:16.886Z] INFO  bobcsi-pc-11044/BROKER: ServiceBroker with 2 service(s) is started successfully.
[2019-04-09T19:10:16.888Z] INFO  bobcsi-pc-11044/TEST: event triggered $broker.started
mol $ call test.emit
mol $ call test.emit
{ options: {}, actionName: 'test.emit' }
>> Call 'test.emit' with params: {}
[2019-04-09T19:10:46.978Z] INFO  bobcsi-pc-11044/TEST: event triggered this.is.an.event
>> Execution time: 4ms
>> Response:
undefined
mol $

@levia7han
Copy link
Author

Sorry, that snippet was a bit misleading. Let me clarify.

This issue happens in integration tests and running live from docker-compose. I have not had any luck reproducing it in a unit test or running local. In fact it will pass all unit tests, which is where I was bitten.

If you check out the repository I linked to, you should see what is going on better. As I have integration tests and the docker-compose setup to repo the issue.

But if I was to create two services putting both into separate docker containers. One with an action that broadcasts an event. The other subscribed to all events ("**"). Then I was to call the first container. (through the repl for instance) I will not see the event fire in the other container.

If I subscribe to the event directly it will work. But the wildcards don't seem to. To further this strangeness; If I subscribe to "**" and "*.*.*.*" And fire off and event "this.is.an.event" I will get two events logged. But then if you remove the "*.*.*.*" event you get no event logged.

@icebob
Copy link
Member

icebob commented Apr 9, 2019

Are you using disableBalancer: true?

@levia7han
Copy link
Author

yes

@icebob
Copy link
Member

icebob commented Apr 10, 2019

Ok, the problem is because disableBalancer. So adding > to matcher function is not a good solution because > wildcard is only NATS specific. We need to handle wildcards handling in NATS transporter code.

I will fix it.

@icebob icebob added Module: Transporter and removed Status: Need reproduce Issue waits for reproducing labels Apr 10, 2019
@icebob icebob added this to the 0.13 milestone Apr 10, 2019
@levia7han
Copy link
Author

Sounds good, I was wondering if there was a better place for it than the match method.
Thank you very much for addressing this.

@icebob icebob closed this as completed in 776d2e4 Apr 10, 2019
@levia7han
Copy link
Author

@icebob Thanks for fixing this for us. We were just wondering when you were planning your next release.

@icebob
Copy link
Member

icebob commented Apr 18, 2019

I've just released 0.13.9

@levia7han
Copy link
Author

You are the best. Thanks

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

No branches or pull requests

2 participants