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

Awaiting ctx.emit behave differently locally and remotely #1065

Open
4 tasks done
Embraser01 opened this issue Feb 24, 2022 · 1 comment
Open
4 tasks done

Awaiting ctx.emit behave differently locally and remotely #1065

Embraser01 opened this issue Feb 24, 2022 · 1 comment
Labels
Type: Discussion Discussion issue about a feature

Comments

@Embraser01
Copy link

🛑 Do you want to ask a question ?

Please head to the Discord chat

Prerequisites

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

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

Current Behavior

When awaiting a ctx.emit call, the promise wait for the event to be completed locally but will resolve as soon as the event is sent on the transit when sent remotely.

A workaround for now is to remove the await but if the emit is wrapped by a middleware, it prevents it to be async.

This can be found here:

if (!this.options.disableBalancer) {
const endpoints = this.registry.events.getBalancedEndpoints(eventName, opts.groups);
// Grouping remote events (reduce the network traffic)
const groupedEP = {};
endpoints.forEach(([ep, group]) => {
if (ep.id === this.nodeID) {
// Local service, call handler
const newCtx = ctx.copy(ep);
promises.push(this.registry.events.callEventHandler(newCtx));
} else {
// Remote service
const e = groupedEP[ep.id];
if (e) e.groups.push(group);
else
groupedEP[ep.id] = {
ep,
groups: [group]
};
}
});
if (this.transit) {
// Remote service
_.forIn(groupedEP, item => {
const newCtx = ctx.copy(item.ep);
newCtx.eventGroups = item.groups;
promises.push(this.transit.sendEvent(newCtx));
});
}
return this.Promise.all(promises);

Expected Behavior

I would expect to have a consistent behavior for both cases. I'd say I'd expect the local event to not wait for completion (matching the remote behavior).

Note: It could break tests with events that relies on this behavior.

Failure Information

Reproduce code snippet

const { ServiceBroker } = require("moleculer");

const broker = new ServiceBroker();

broker.createService({
  name: "greeter",
  actions: {
    async hello(ctx) {
      const start = Date.now();
      await ctx.emit("my-event", { test: 1 });
      this.logger.warn(`Event emit took ${Date.now() - start}ms`);
    }
  },
  events: {
    "my-event": async () => {
      await new Promise((resolve) => setTimeout(resolve, 5000));
    }
  }
});

broker.start().then(() => {
  broker
    .call("greeter.hello", { name: "CodeSandbox" })
    .catch((err) => broker.logger.error(err.message));
});

Gives an output like this:
image

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: 0.14.19
  • NodeJS version: LTS
  • Operating System: Linux, Windows, MacOS
@icebob icebob added the Type: Discussion Discussion issue about a feature label Apr 19, 2022
@intech
Copy link
Member

intech commented May 29, 2022

@icebob This one affects emit and broadcast with an enabled balancer, which becomes a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Discussion Discussion issue about a feature
Projects
None yet
Development

No branches or pull requests

3 participants