moleculerjs / moleculer-channels

Reliable messages for Moleculer services via external queue/channel/topic.
MIT License
75 stars 15 forks source link

fix: enforce buffer type on data passed to serializer #58

Closed dowster closed 1 year ago

dowster commented 1 year ago

The NATS client library can either return a Buffer or a Uint8Array (which Buffer extends). However, the serializer only expects to work with a Buffer. If passed a Uint8Array the deserialize function will fail.

dowster commented 1 year ago

@icebob this is the first moleculer-channels PR I've put in. Can you let me know if it needs more documentation or testing? I wasn't totally sure what to include here.

dowster commented 1 year ago

Locally we were running traffic through the JetStream queue. I'm honestly not sure how the nats package decides if it will return a Buffer or a Uint8Array. I could try calling the function with Buffer and Uint8Array and see if it throws in either case.

dowster commented 1 year ago

Hey @icebob, I just wanted to ask if there's anything else needed from me to move this forward.

icebob commented 1 year ago

No, sorry it's waiting for me to test it in my env.

icebob commented 1 year ago

I've just checked your PR with benchmark. It looks the isBuffer is an expensive method. If I skip it and use always Buffer.from is faster, so I changed your code a little bit.

Original code in PR: image

My modified code (+7%): image

icebob commented 1 year ago

Released in 0.1.5