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

SNOW-889050: NextJS Weird Warning Message #609

Closed
detailedghost opened this issue Aug 8, 2023 · 7 comments
Closed

SNOW-889050: NextJS Weird Warning Message #609

detailedghost opened this issue Aug 8, 2023 · 7 comments
Assignees
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@detailedghost
Copy link

Summary

Working with an NX monorepo with NextJS. I'm able to query the database, but grabbing a weird warning message when I launch the application. Figured it might be worth flagging, sorry if there's a duplicate issue. Regardless, I appreciate any response.

Requested info

  1. What version of NodeJS driver are you using?
    • v18.16.0
  2. What operating system and processor architecture are you using?
    • Windows 10 with WSL 2 Ubuntu
  3. What version of NodeJS are you using?
    • v18.16.0
  4. What are the component versions in the environment (npm list)?
├── @nx/cypress@16.6.0
├── @nx/eslint-plugin@16.6.0
├── @nx/jest@16.6.0
├── @nx/js@16.6.0
├── @nx/linter@16.5.5
├── @nx/next@16.6.0
├── @nx/react@16.5.5
├── @nx/workspace@16.6.0
├── @radix-ui/react-navigation-menu@1.1.3
├── @radix-ui/react-slot@1.0.2
├── @swc/cli@0.1.62
├── @swc/core@1.3.75
├── @swc/helpers@0.5.1
├── @tanstack/react-query@4.32.6
├── @testing-library/react@14.0.0
├── @types/jest@29.5.3
├── @types/node-fetch@2.6.4
├── @types/node@20.4.8
├── @types/react-dom@18.2.7
├── @types/react@18.2.18
├── @types/snowflake-sdk@1.6.13
├── @typescript-eslint/eslint-plugin@5.62.0
├── @typescript-eslint/parser@5.62.0
├── autoprefixer@10.4.14
├── babel-jest@29.6.2
├── class-variance-authority@0.7.0
├── clsx@2.0.0
├── cypress@4.12.1
├── eslint-config-next@13.4.12
├── eslint-config-prettier@8.1.0
├── eslint-plugin-cypress@2.14.0
├── eslint-plugin-import@2.28.0
├── eslint-plugin-jsx-a11y@6.7.1
├── eslint-plugin-react-hooks@4.6.0
├── eslint-plugin-react@7.32.2
├── eslint@8.46.0
├── jest-environment-jsdom@29.6.2
├── jest-environment-node@29.6.2
├── jest@29.6.2
├── lucide-react@0.263.1
├── next@13.4.1
├── node-fetch@3.3.2
├── nx-cloud@16.2.0
├── nx@16.5.5
├── postcss@8.4.27
├── prettier@3.0.1
├── prisma@5.1.1
├── proxy-agent@5.0.0
├── react-dom@18.2.0
├── react-hook-form@7.45.4
├── react@18.2.0
├── snowflake-sdk@1.7.0
├── tailwind-merge@1.14.0
├── tailwindcss-animate@1.0.6
├── tailwindcss@3.3.3
├── ts-jest@29.1.1
├── ts-node@10.9.1
├── tslib@2.6.1
├── typescript@5.1.6
└── zod@3.21.4

7.Server version:*
- 7.27.1

  1. What did you do?

    • Install the snowflake-sdk
    • Connected and queried a test database
  2. What did you expect to see?

  • No warning messages when launching NextJs app
  1. What should have happened and what happened instead?
  • No strange NextJS warning messages
  1. Can you set logging to DEBUG and collect the logs?

  2. What is your Snowflake account identifier, if any? (Optional)

    • SLNASYY-KI87678
@detailedghost detailedghost added the bug Something isn't working label Aug 8, 2023
@github-actions github-actions bot changed the title NextJS Weird Warning Message SNOW-889050: NextJS Weird Warning Message Aug 8, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Aug 9, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage Issue is under initial triage label Aug 9, 2023
@sfc-gh-dszmolka
Copy link
Collaborator

hi and thank you for raising this issue. it looks like similar issues are all over the place, for example

and so on, even outside of snowflake-sdk. Seems to be originating from encoding being an optional dependency for node-fetch (which latter module is an indirect dependency for snowflake-sdk) and as you mentioned, is only a warning message and does not cause any actual issues.

as a workaround for now, it should go away if you install encoding with npm i -D encoding (or npm i encoding). I'll also check how we could get rid of it without needing the workaround.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-in_progress Issue is worked on by the driver team and removed status-triage Issue is under initial triage labels Aug 9, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka linked a pull request Aug 9, 2023 that will close this issue
@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review status-in_progress Issue is worked on by the driver team and removed status-in_progress Issue is worked on by the driver team status-pr_pending_merge A PR is made and is under review labels Aug 9, 2023
@sfc-gh-dszmolka
Copy link
Collaborator

before trying to solve this issue in the snowflake-sdk package, I wanted to see it myself that adding encoding to the dev-dependencies will work. (since node-fetch v2 requires it as peer-dependency but nobody provides it)

i don't have much experience with Next.JS, so following the docs I started setting up a new Next.js project from scratch, excluded some node-only stuff from webpack config in next.config.js (fs, child_process, tls, net, dns, async_hooks, module) and after I got past this phase, I faced the following error:

Module not found: Can't resolve 'proxy-agent'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/urllib/lib/urllib.js
./node_modules/urllib/lib/index.js
./node_modules/snowflake-sdk/lib/http/base.js
./node_modules/snowflake-sdk/lib/http/node.js
./node_modules/snowflake-sdk/lib/snowflake.js
./node_modules/snowflake-sdk/index.js
./pages/snowflake.js

which is a different module missing from one of the dependencies of snowflake-sdk, but exactly the same situation as I see it: similarly to how node-fetch v2 requires encoding as a peer-dependency; so does urllib require proxy-agent as a peer-dependency:

  "peerDependencies": {
    "proxy-agent": "^5.0.0"
  },
  "peerDependenciesMeta": {
    "proxy-agent": {
      "optional": true
    }
  },

and in both cases, nobody provides this to them (encoding to node-fetch v2, and proxy-agent to urllib).

Since situation seems to be very similar, but with different modules, I'm wondering how is the missing proxy-agent problem solved in your Next.js setup today? Since it doesn't seem to be missing in your case.
I'm wondering , why isn't it missing and how is encoding different which is also a peer-dependency for another module (and not provided by anything).

@sfc-gh-dszmolka sfc-gh-dszmolka added status-information_needed Additional information is required from the reporter and removed bug Something isn't working labels Aug 9, 2023
@sfc-gh-dszmolka
Copy link
Collaborator

okay I think I found something. apparently just very recently urllib released a fresh release 2.41.0 to fix a critical vulnerability, which was caused by one of their indirect dependencies, vm2 (needed by proxy-agent)

the dev over there moved proxy-agent (which introduces vm2 which has a critical, unfixed vulnerability) into peer-dependencies - and at the same time, they provide proxy-agent it as dev-dependency. see PR#457 in https://github.com/node-modules/urllib

before we could move on with a similar approach, I really would like to see it working, before we introduce a new dev- dependency into our module which will be probably unused anyways - to get rid of a warning.

Do you think it would be possible to provide a minimal viable reproduction Next.Js app, which when run, exhibits the error that only encoding is missing ?

@sfc-gh-dszmolka sfc-gh-dszmolka added bug Something isn't working and removed status-in_progress Issue is worked on by the driver team labels Aug 9, 2023
@detailedghost
Copy link
Author

@sfc-gh-dszmolka Thanks for the investigation. The good news is that the package does work, I'm able to query the database just fine, just the weird warn message is spit out at the beginning.

I took the dumb approach and just installed "proxy-agent" and that seemed to get me a simple warning message. Doing a regular next app seems to break down, and complain about the proxy agent. I can go down the rabbit hole and try to manually install every needed dependency but certainly isn't ideal. Is there a flag to hide the warn message?

@sfc-gh-dszmolka
Copy link
Collaborator

i'm not entirely sure about what options are in next.js to hide the warning messages, but some web search showed these hits:

stats: {
  warnings: false
}
webpack: (config) => {
  config.resolve.fallback = { "aws-crt": false };

  return config;
},

hope some of them helps.
the issue seems to be that some direct or indirect dependencies put certain packages in their peer-dependencies

  • urllib v2 with proxy-agent
  • node-fetch v2 with encoding

urllib seems to satisfy it for itself in dev-dependencies, but node-fetch does not from anywhere. Therefore the workaround seems to be either to install these packages as dev-dependency (as it worked for multiple people, see Issues from the original response), or -and i did not test this- try and suppress the warning or configure the fallback. Again, I'm not very familiar with Next.JS but hope this makes sense. Perhaps there are other ways too to hide this warning message in Next.JS. There is no such issue in Node.JS for which this driver is built and tested against.

@sfc-gh-dszmolka sfc-gh-dszmolka removed the status-information_needed Additional information is required from the reporter label Aug 10, 2023
@detailedghost
Copy link
Author

Wanted to follow back up. Still exhibiting the warning message, even with adding in encoding, proxy-agent, coffee-script and a few others in as dev dependencies.

Also tried route 2 with attempting to suppress the issue via next.config.js as suggested, but it appears the import error message is outside the scope of NextJS.

I did notice that vm2 lib flags users to migrate to isolated-vm. Could that possibly be the issue?

@sfc-gh-dszmolka
Copy link
Collaborator

I'm really not sure anymore if we should address this Next.JS related issue here. This connector is built for and tested on Node.JS - and there's no such issues on Node.JS

  1. For the original issue (missing encoding emits a warning), please consider filing an issue against node-fetch the warning comes from there. (encoding peer-depended on optionally, but not provided)

  2. For the newer issue (missing stuff like proxy-agent et. al), it comes from urllib v2, since they moved vulnerable vm2 to peer-dependencies.
    This solves a critical security issue for a lot of users (CVE-2023-37466, CVE-2023-37903), but seems to provide a console-emitted warning for you.
    You can just ignore these warnings. In the long term, they should automatically be gone once we manage to move on to urllib v3, because it does not depend on proxy-agent.

Since this issue does not exist in Node.JS, I'm marking this one as closed but if anyone stumbles upon it and has a solution which can make the harmless warnings disappear in Next.JS, they can share it in a comment.

@sfc-gh-dszmolka sfc-gh-dszmolka closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the wontfix This will not be worked on label Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants