moleculer

moleculer copied to clipboard

:memo: Description

:dart: Relevant issues

https://discord.com/channels/585148559155003392/585149564135145493/955759619207225384 With io, I'm calling $node.actions . And it crash with : Cannot read property 'onlyLocal' of null coming from : https://github.com/moleculerjs/moleculer/blob/422876c8f0142caa313a792f8069ac7aac12fb47/src/registry/action-catalog.js#L131

Debugging a little, I found this : const res = check(ctx.params != null ? ctx.params : {}, { meta: ctx }); => (this will define the default params of an action ) : https://github.com/moleculerjs/moleculer/blob/422876c8f0142caa313a792f8069ac7aac12fb47/src/validators/base.js#L102 .

The problem here ( I think ) . Is the ctx.params != null ? ctx.params : {} => yes, it will pass an object to the check function, but didn't retrieve the default params ?

And so, If I correctly understand how it works, const params = ctx.params != null ? ctx.params : {} const res = check(params, { meta: ctx }); ctx.params = params;

will resolve this error ? ( on my setup => yes )

:gem: Type of change

• [X] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

:scroll: Example code

try {
    const res = await broker.call('$node.actions', null);
    console.log(res);
} catch (e) {
    console.log(e);
}

:vertical_traffic_light: How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• [X] Tested manually on my setup (with example code) + io request Any ideas of tests ?

:checkered_flag: Checklist:

• [ ] My code follows the style guidelines of this project
• [X] I have performed a self-review of my own code
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] New and existing unit tests pass locally with my changes
• [ ] I have commented my code, particularly in hard-to-understand areas

CodeSee Review Map:

Review in an interactive map

View more CodeSee Maps

Legend

@icebob any news / blocking on this ?

I'm not sure it's the good solution, because it changes the logic. E.g. we can't pass null as a value of ctx.params. I mean you call broker.call("hello.world", null) and in your service the ctx.params will be {} instead of null.

I'm not sure it's the good solution, because it changes the logic. E.g. we can't pass null as a value of ctx.params. I mean you call broker.call("hello.world", null) and in your service the ctx.params will be {} instead of null.

Hum, yes, I understand ... But, what about the default from validator ?

~~As far as I remember, the problem is what if I didn't pass paremeters (I need to recheck this) ? ( so ctx.params === undefined / ctx.params == null ) We can do a check and fill with an object if it's == null but !== undefined ?~~ Edit : passing undefined will be replace by {} on first line of serviceBroker.call, not valid

---

Ok I remember . In fact, the problem is here (the PR) . Or in each commands, you need to handle that params are not always an object . And default can be ignored .

Example here : https://github.com/moleculerjs/moleculer/blob/422876c8f0142caa313a792f8069ac7aac12fb47/src/registry/action-catalog.js#L131-L136 You are extracting some values from the params ? but the params can be null, so the protection you set here : https://github.com/moleculerjs/moleculer/blob/422876c8f0142caa313a792f8069ac7aac12fb47/src/internals.js#L69 . Doesn't protect against a call passing null

So, doesn't know how to handle this :

• a validator configuration that will force non null here ? https://github.com/moleculerjs/moleculer/blob/422876c8f0142caa313a792f8069ac7aac12fb47/src/internals.js#L69
• don't allow null if defaults are presents ? ( like this PR, but need to check params modified by the validator )
• validate params before calling another service

In my opinion, the last option is too "risky" . and need lot of works .

I don't know if we can do the first one ?

else, did you have an idea ?

( I need to reopen this, but not with the master branch ) .