DeprecationWarning: Buffer()

[vdabar]

Describe the bug
When I initialize launch-darkly-sdk-client I keep getting this error:
"[DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead."

To reproduce

import * as LaunchDarkly from 'launchdarkly-node-server-sdk';
import DynamoDBFeatureStore from 'launchdarkly-node-server-sdk-dynamodb';

let ldClient: LaunchDarkly.LDClient;

export async function init(launchDarklyStoreTable: string, launchDarklySdkKey: string) {
  const store = DynamoDBFeatureStore(launchDarklyStoreTable);
  const config = {
    featureStore: store,
    useLdd: true
  };
  ldClient = LaunchDarkly.init(launchDarklySdkKey, config);
  await ldClient.waitForInitialization();
}

Expected behavior
Not to print DeprecationWarning: Buffer() error

SDK version
5.13.0

Language version, developer tools
node v10.18.1

Additional context
I believe this error comes from Redis dependency, which needs to be updated to 3.x.x version

[eli-darkly]

I believe this error comes from Redis dependency, which needs to be updated to 3.x.x version

I'm not sure how that could be the reason, if you're seeing this warning at runtime rather than at build time and you're not actually using Redis.

[eli-darkly]

Also, I've been unable to reproduce this behavior. I am running in Node 10.18.1, with launchdarkly-node-server-sdk 5.13.0 and launchdarkly-node-server-sdk-dynamodb 1.1.9. The only difference between my test code and yours is that mine isn't in TypeScript, because I wanted to keep the dependencies as minimal as possible. In any case, I don't get any deprecation warning when I initialize the client.

[eli-darkly]

I also would not expect any database integration code to execute in this particular test code. You are setting useLdd: true, which means it will not try to connect to LaunchDarkly at all, it will just read flags as needed from the database - but in our example, you're not evaluating any flags. Therefore it's not going to do any DynamoDB operations— you'll see this if you set launchDarklyStoreTable to a nonexistent table name; it makes no difference, because it's not actually trying to do anything with the table.

But even if I change this so that it does evaluate a flag, I don't get any deprecation warning,.

[vdabar]

@eli-darkly thx for quick reply, apparently it happens when it's bundled with webpack. I have prepared an example: https://github.com/vdabar/launch-darkly-webpack-example
In order to replicate it run those commands:

cd launch-darkly-webpack-example
npm install
npm run build
node dist/main.js

[eli-darkly]

I'm not sure why you're using webpack for Node code (the server-side Node SDK should never be run in a browser), but that does explain why you'd be seeing the warning only in that context.

As documented (see the "History" sections for the deprecated constructors), there's some logic that suppresses the warning if the constructor is being called from anywhere inside node_modules. I assume that the rationale was that lots and lots of packages still used those constructors, so application developers might not really have the option of eliminating all such usages that might be in transitive dependencies and so the warning would just be useless and annoying; in other words, they just wanted to encourage application developers to get rid of those usages in their own code. But, in the webpacked version of the code, those dependencies aren't in node_modules any more and so you see the warning.

Again, I don't know why you've assumed that the Redis code is to blame. It's true that the 2.x driver includes deprecated usages, and it does make sense for us to update that package... but that code is not executing in your program at all, and there's no mechanism for its mere presence in the source file to cause the warning to appear. I think it's much more likely that another dependency is to blame: grep -r 'new Buffer(' node_modules shows matches in at least 11 non-Redis packages (not including packages that are just brought in by webpack). I'd have to take a closer look to see which ones are more likely to be actually in use, but my point is that I think even if we updated the Redis dependency immediately, it would probably not make this warning go away. And, given that we have only limited control of transitive dependencies, it may or may not be possible to completely eliminate the warning even if we update to the latest versions of everything we're using. But I'll look into it.

[eli-darkly]

To test that theory, I modified a local copy of the Node SDK to update the redis package to 3.0.2, rebuilt your example app using that modified SDK, and reran the build and the app. The same warning appeared.

[vdabar]

We have monorepo setup for serverless functions and to prepare small bundles we use webpack with parameter target: node. if you also install asn1.js version 5.3 as a dependency it should fix all the warnings. I added a working example with updates packages in the same repo just install packages with npm ci. For sure this solution wouldn't solve the problem permanently but at least it should be fine until next function deprecation.

[eli-darkly]

@vdabar Could you say more about how you determined that asn1.js was the package you should add an explicit dependency for? That is not a dependency of the Node SDK at all, not even a distant transitive dependency, as far as I can tell.

[eli-darkly]

It's possible that the reason this is working for you is that one of its transitive dependencies is the thing we actually need to update - but if so, we should do specifically that, not introduce a new dependency on something we don't otherwise need.

[vdabar]

What I wanted to say that in launchdarkly-node-server-sdk package the only one dependency still using deprecated Buffer is Redis. Asn1.js dependency comes from webpack and it's just a resolution hack to show you that bump of the Redis version fixes issue with your package.

[eli-darkly]

What I wanted to say that in launchdarkly-node-server-sdk package the only one dependency still using deprecated Buffer is Redis. Asn1.js dependency comes from webpack and it's just a resolution hack to show you that bump of the Redis version fixes issue with your package.

I'm not sure what the difficulty in communication is here and I'm sorry if I was unclear, but as I tried to say earlier:

1. It is not true that the only dependency still using a deprecated Buffer constructor is Redis. This can be easily verified by the grep search that I mentioned.

2. It is impossible for any code in the redis package to be executed in your test scenario, so that package by itself cannot be the cause of the warning you saw. And I proved this by updating the redis version in the SDK, rebuilding your test app using that hacked SDK, and observing the same warning.

Both of those things are true to the best of my knowledge... so I'm not sure why you answered my question about the asn1.js package by telling me once again that this is all about Redis.

Again, asn1.js is not a direct dependency of our SDK, and it is not a transitive dependency either. You can easily verify that with npm ls. (There is a completely different package called just asn1, but it is only a dev dependency.) So this is why I asked: "Could you say more about how you determined that asn1.js was the package you should add an explicit dependency for?" In other words, what was your process for coming up with that solution, because it seems fairly arbitrary to me. When you say "asn1.js dependency comes from webpack and it's just a resolution hack", it's not clear to me what you mean by that in this context.

The other problem is that I cannot get your solution to work for me at all. That is:

• I took the latest version of your test repo that has both the new asn1.js dependency and the updated redis dependency, and checked it out in a new directory to be sure that I was doing a completely clean build. This is in Node 10.18.1.
• npm install fails: No matching version found for @types/[email protected].

[eli-darkly]

I mean, if I do npm ci instead of npm install, then it does work, but that just means that the build will only work as long as your package-lock.json stays the same— in other words, what you have in package.json is not adequate for building the project from scratch. So that is not a usable solution.

[vdabar]

Of course, it won't work with npm install, because I manually bumped up Redis version in launch-darkly-server-sdk via package-lock.json. grep search method found all deprecated values in other dependencies such as webpack in this case. However, if you search only in your package tree there are no other packages that use deprecated Buffer. Also, as you said asn1.js is not a direct dependency of your SDK, it's a transitive dependency of webpack and it's just added to resolve the same issue from other packages.

[vdabar]

I understand that given code snippet never reaches that code, however, since your package makes the switch between different stores so easy it could change anytime soon and it would be nice to have the latest dependencies with as little deprecated code as possible.

[eli-darkly]

I understand that given code snippet never reaches that code, however, since your package makes the switch between different stores so easy it could change anytime soon and it would be nice to have the latest dependencies with as little deprecated code as possible.

I feel like this conversation is going around in circles a bit. I've already said several times that yes, it would be a good idea for us to update the Redis package. The point I was making was that the entire premise of the issue you filed— that the warning you were seeing was due to the Redis package— is false, and all of the test cases you proposed, including the last one, are still going on that wrong premise.

When you updated your test repo and said "Asn1.js dependency comes from webpack and it's just a resolution hack to show you that bump of the Redis version fixes issue with your package", you weren't actually showing any such thing. If you had not updated Redis in that test, but only updated asn1.js, the warning would have still gone away, because none of the Redis code is executing in your test. You were simply testing that you had made warnings from webpack dependencies go away.

Similarly, after we update the Redis version, I do not think those warnings are going to go away for you unless you continue to use your webpack dependency hack. That part is something we can't control. And the approach that you used— manually editing only package-lock.json— is not something we can use, for reasons that should be obvious; when trying to illustrate a solution to a problem, it's usually better to use a technique that the other person can actually reproduce.

But in any case I think we've discussed this enough and the basic point is clear.

[eli-darkly]

[tracked internally as 74036]

[eli-darkly]

I should mention that one potential issue with updating to redis 3.x, although I think in practice it will not be a big deal, is that our SDK's Redis integration allows you to pass in an existing Redis client object. So there may be an application that is currently using redis 2.x, and passes a 2.x Redis client into the SDK which internally depends on 3.x. But as far as I can tell, none of the breaking changes in the 3.x API should affect our own fairly simple usage of the package. If it turns out that they do, then we would need to postpone this update until the next major version release of the SDK when breaking changes would be OK.

[eli-darkly]

The Redis driver has been updated in the 5.13.1 release. This should not affect any existing applications, since none of the Redis APIs that we use have changed.