Buffer type check is broken after upgrade rsocket.js to 0.0.22

[yingjieg]

rsocket.js introduces new LiteBuffer implementation in recent versions, but in file /rsocket-core/src/RSocketEncoding.js, it still use the raw Buffer.isBuffer to check the type, this change break the working code based on 0.0.19.

Steps to Reproduce

Use BufferEncoders as following:

  const transport = new RSocketWebSocketClient(
    {
      url: getSocketURL(),
      wsCreator: (url: string) => new WebSocket(url),
      debug: true,
    },
    BufferEncoders,
  );

then you will get the error message raised by RSocketEncoding.js

RSocketEncoding: Expected value to be a buffer,

Possible Solution

Update the Buffer.isBuffer check to compatible with LiteBuffer.

Environment

• RSocket version(s) used: 0.0.22

[OlegDokuka]

Hi @yingjieg!

Thank you for rising that!
Unfortunatelly I can not reproduce this issue with my setup (https://github.com/OlegDokuka/rsocket-crosslanguage-example/blob/master/js-client/package.json) can you please provide more details or a sample project which reproduces that?

Cheers,
Oleh

[OlegDokuka]

Alright. Reproduced locally. Looking into that.

Thanks,
Oleh

[OlegDokuka]

After some investigation, it was figured out that it is not related to rsocket but rather related to webpack setup (assume the bundler used is webpack).

The problem is that by default webpack uses polyfills if it finds any node dependencies and bundles them all the way to the browser (for more info see -> https://stackoverflow.com/questions/40467141/webpack2-node-libs-browser-exclude).

Previously, it happened that our Buffer implementation was directly derived from a library used in webpack-lib-browser, hence - no issues appeared before. Nevertheless, in the newer version or rsocket-js, we have rewritten completely that implementation and now it does not intersect with the one bundled by webpack. Therefore, once different Buffer implementations meet at Buffer.isBuffer it results in an obvious false outcome. To fix that, you have to disable webpack-node-libs putting the following property into the config

const webpackConfig = {
  node: false
}

please see the following project for the reference -> https://github.com/OlegDokuka/rsocket-crosslanguage-example

[yingjieg]

Hi @OlegDokuka

Thank you for investigating this issue. you are right, the root cause is the conflict of polyfill Buffer implementation and LiteBuffer, but I think disable webpack-node-libs is not a good choice, lots of UI projects based on polyfill Buffer implementation. Like our project, it heavily depends on Buffer base64 encode/decode.

After investigation, I think following solutions might work.

Make sure Buffer.isBuffer import the LiteBuffer implementation in RSocketEncoding.js

 import type {Encodable} from 'rsocket-types';

// add this import in rsocket-core/src/RSocketEncoding.js
+import {LiteBuffer as Buffer} from './LiteBuffer'; 

then we can import toBuffer, createBuffer from rsocket-core

  import { toBuffer, createBuffer } from 'rsocket-core';

  return encodeAndAddWellKnownMetadata(
    encodeAndAddCustomMetadata(
      createBuffer(0),
      'message/x.rsocket.authentication.bearer.v0',
      toBuffer(token),
    ),
    MESSAGE_RSOCKET_ROUTING,
    toBuffer(String.fromCharCode(ROUTE.length) + ROUTE),
  );

or we can inject LiteBuffer in webpack:

  new webpack.ProvidePlugin({
    Buffer: ['buffer', 'Buffer'],
    LiteBuffer: ['rsocket-core/build/LiteBuffer.js', 'LiteBuffer'],
  }),

export function getMetadata(token: string) {
  return encodeAndAddWellKnownMetadata(
    encodeAndAddCustomMetadata(
      LiteBuffer.alloc(0),
      'message/x.rsocket.authentication.bearer.v0',
      LiteBuffer.from(token),
    ),
    MESSAGE_RSOCKET_ROUTING,
    LiteBuffer.from(String.fromCharCode(ROUTE.length) + ROUTE),
  );
}

What do you think ?

BR
Yingjie

[OlegDokuka]

@yingjieg There are 3 options which we can follow:

1. We can take the LiteBuffer module off the core to a separate module and one can decide how to go (this is a breaking change)
2. We can go away from Node buffer and create our own Data representation (another breaking change that does not reduce complexity)
3. We keep it as it is now and since node-libs-browser is deprecated a user has to decide which polyfills to take into the project.

Definitely, I'm for the 3d option. Noteworthy, the webpack-libs-browser was deprecated for a reason which is - "user has to decide what to do on its own" and not Webpack has to do some magic inclusion.

We will consider the first option to give more flexibility and avoid any limitations from our side, but that will happen not earlier than in 0.1.x or 1.0.0.

[yingjieg]

@OlegDokuka

Thanks to explain the 3 options. I will remove polyfill part in our project and import specific "Buffer" implementation to solve this issue. The first option is cool, looking forward to see the enhancement in the future.

Thank you!

BR
Yingjie

[OlegDokuka]

@yingjieg what you need to do is to exclude Buffer polyfill brought by Webpack. We automatically import our LiteBuffer implementation as a global scope Buffer (via the window scope). You don't need to import anything explicitly

[yingjieg]

@OlegDokuka In our project, we use lots of buffer base64 encode/decode operations, but there is no implementation in LiteBuffer.

const notImplementedEncodings = [
  'base64',
  'hex',
  'ascii',
  'binary',
  'latin1',
  'ucs2',
  'utf16le',
];

[OlegDokuka]

Sounds reasonable. I will add a separate issue on moving lite-buffer to a separate module

[myklt]

Hi @OlegDokuka ,

I've faced the same issue, but unfortunately do not have control of webpack default polyfills (using create-react-app). Could you please give me a hint, why the proposed solution by @yingjieg to use LiteBuffer.isBuffer in RSocketEncoding.js wouldn't work?

For now, a very quick and dirty solution was to set buffer._isBuffer = true to created composite buffer using RSocket utils.

Regards,
Mykolas.

[OlegDokuka]

@myklt as I mentioned, Webpack sets its own polyfills which are incompatible with LiteBuffer.

Looks like it is a common issue for the webpack users. I will try to make a workaround. Stay tuned

Cheers,
Oleh

[lempikaSolutions]

I had the same issue that @MyKit and indeed the solution that he proposed works for me !!!
const metadataBuffer = encodeCompositeMetadata([
[TEXT_PLAIN, Buffer.from('Hello World')],
[MESSAGE_RSOCKET_ROUTING, () => encodeRoute(getTokenRoute)],
[MESSAGE_RSOCKET_AUTHENTICATION, () => encodeSimpleAuthMetadata(username, password)]
]);
// eslint-disable-next-line no-underscore-dangle
metadataBuffer._isBuffer = true;

[myklt]

@OlegDokuka , thanks for the fix! Upgraded to 0.0.23 and it works as expected now.

[myklt]

Apparently I got happy too early. It works perfectly while running locally, but something wrong happens, when the production bundle is built: two type of Buffer objects are being created & compared again. I will investigate it further next week, how to avoid the Buffer from Webpack being used.

[myklt]

Update: I was able to solve the issue by using toBuffer from rsocket-core instead of using global Buffer.from in data serializer. I think it would be a good idea to use RSocket compatible buffer implementation explicitly everywhere. I think solving this issue #112 should help to avoid such errors I've faced.

[OlegDokuka]

@myklt yeah, I dont wanna require anyone to use LiteBuffer. In the meanwhile, if you have an example that reproducers the issues with your setup - don't hesitate to send it to me

[myklt]

@OlegDokuka sorry for the delay. Here is a minimalistic example using create-react-app: https://github.com/myklt/cra-rsocket .

[kkojot]

After updating rsocket to 0.0.23 my resumable connection ended up with an endless loop :) Locally everything worked fine (webpack) but after generating static pages (with vue and nuxt) and hosting them on apache I saw {error: Invariant Violation: RSocketEncoding: Expected value to be a buffer, got.... error.
The proposed solution here #110 (comment) helped me to solve the issue (using createBuffer() and toBuffer() instead of Buffer.aloc() and Buffer.from())

[OlegDokuka]

@myklt, @kkojot @lempikaSolutions @yingjieg I did a temporary fix in 0.0.25 which should eliminate that bad experience you all faced before. I recommend updating on 0.0.25 as soon as possible.

@myklt I did a check with 0.0.25 and the app you mentioned before and everything worked like a charm!

[myklt]

Hi @OlegDokuka! Thanks a lot, I've just tried it out and it works like a charm!

[SerCeMan]

We've noticed the same issue with 0.0.25, some of the third-party libraries managed to pick up a polyfil and started failing with Error: Not implemented: "base64" encoding.

[ascpeter]

I'm getting a buffer warning like this:
RSocketEncoding: Expected value to be a buffer, got '�����route-here'

Strangely enough, this does NOT occur if using webpack's devtool: 'eval'. But it happens with anything else. Consequently, it works in dev but breaks in production. :(

This is with rsocket-core 0.0.25.

[ascpeter]

The _isBuffer appears to work for me.

[zur4ik]

Getting this error in 0.0.27

Invariant Violation: RSocketEncoding: Expected value to be a buffer, got `)message/x.rsocket.authentication.bearer.v0...

Using CRA and there are no errors in dev mode, only in production builds

[roie16]

Hi,
I have experienced this issue with rsocket 0.0.27
as this is a temp demo project I looked for a quick workaround, and I fixed it by just added the following dependency to package.json:
"node-polyfill-webpack-plugin": "^2.0.1"
leaving it here in case someone will need a temp quick workaround