Skip to content
This repository has been archived by the owner on May 10, 2021. It is now read-only.

"The 'path' argument must be of type string. Received undefined" when using next-auth #168

Closed
zaarheed opened this issue Feb 14, 2021 · 18 comments
Labels
next-auth related to next-auth priority: medium type: bug code to address defects in shipped code

Comments

@zaarheed
Copy link

Describe the bug
I initially raised a bug on the next-auth repo here: nextauthjs/next-auth#1311

I am using next-on-netlify with next-auth. I am able to enter my email address at /api/auth/signin, but the following page (/api/auth/signin/email) shows the following runtime error:

{"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"TypeError [ERR_INVALID_ARG_TYPE]: The \"path\" argument must be of type string. Received undefined","trace":["Runtime.UnhandledPromiseRejection: TypeError [ERR_INVALID_ARG_TYPE]: The \"path\" argument must be of type string. Received undefined","    at process.<anonymous> (/var/runtime/index.js:35:15)","    at process.emit (events.js:314:20)","    at processPromiseRejections (internal/process/promises.js:209:33)","    at processTicksAndRejections (internal/process/task_queues.js:98:32)"]}

The full logs on Netlify show the following:

11:22:40 PM: 2021-02-13T23:22:40.108Z	b4c355c3-801b-4f2e-b1a4-9c1fc1f1ef88	INFO	[request] /api/auth/signin

11:22:40 PM: Duration: 25.63 ms	Memory Usage: 133 MB	Init Duration: 632.42 ms	

11:22:45 PM: 2021-02-13T23:22:45.318Z	754432c8-0294-4ed2-b94e-09ee1c1657bb	INFO	[request] /api/auth/signin/email

11:22:45 PM: 2021-02-13T23:22:45.825Z	754432c8-0294-4ed2-b94e-09ee1c1657bb	WARN	Warning: no saslprep library specified. Passwords will not be sanitized

11:22:46 PM: 2021-02-13T23:22:46.115Z	754432c8-0294-4ed2-b94e-09ee1c1657bb	ERROR	Unhandled Promise Rejection 	{"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"TypeError [ERR_INVALID_ARG_TYPE]: The \"path\" argument must be of type string. Received undefined","reason":{"errorType":"TypeError","errorMessage":"The \"path\" argument must be of type string. Received undefined","code":"ERR_INVALID_ARG_TYPE","stack":["TypeError [ERR_INVALID_ARG_TYPE]: The \"path\" argument must be of type string. Received undefined","    at validateString (internal/validators.js:120:11)","    at Object.dirname (path.js:1128:5)","    at find_package_json (/var/task/src/nextPage.js:132745:23)","    at find_package_json_with_name (/var/task/src/nextPage.js:132762:20)","    at require_optional (/var/task/src/nextPage.js:132797:13)","    at Object.<anonymous> (/var/task/src/nextPage.js:153355:53)","    at Generator.next (<anonymous>)","    at asyncGeneratorStep (/var/task/src/nextPage.js:153264:103)","    at _next (/var/task/src/nextPage.js:153266:194)","    at processTicksAndRejections (internal/process/task_queues.js:97:5)"]},"promise":{},"stack":["Runtime.UnhandledPromiseRejection: TypeError [ERR_INVALID_ARG_TYPE]: The \"path\" argument must be of type string. Received undefined","    at process.<anonymous> (/var/runtime/index.js:35:15)","    at process.emit (events.js:314:20)","    at processPromiseRejections (internal/process/promises.js:209:33)","    at processTicksAndRejections (internal/process/task_queues.js:98:32)"]}

11:22:46 PM: Duration: 805.37 ms	Memory Usage: 136 MB	

11:22:46 PM: Unknown application error occurred

I can't make much of what's going on, but digging through node_modules it seems the validateString() method mentioned in the logs above belongs to the Next.js library.

Any ideas on whether this is a next issue, next-auth issue, or a next-on-netlify issue?

To Reproduce
Steps to reproduce:

  1. Navigate to https://next-on-netlify-auth.netlify.app/api/auth/signin
  2. Enter email
  3. See error message

The same steps above work without error on localhost (both npm run dev and netlify dev).

Here is a repo with the code: https://github.com/zaarheed/next-on-netlify-auth. Run it locally or deploy to Netlify. Remember to add environment variables in .env (example provided) or in the Netlify dashboard.

Versions

"next": "10.0.6"
"next-auth": "3.4.1"
 "next-on-netlify": "2.8.7"
@lindsaylevine
Copy link
Contributor

lindsaylevine commented Feb 15, 2021

@zaarheed hey! thanks so much for providing such a detailed issue. i wish it was enough but unfortunately i need a little bit more. i'd love to help you but don't have the time to set up the env vars that seem to be needed/legitimate in order to exactly repro what you're seeing. if you can provide valid "mock" values for EMAIL_FROM, EMAIL_SERVER_ADDRESS and IDENTITY_DB_URL, i can handle NEXTAUTH_URL and NEXT_PUBLIC_BASE_URL (obv, as they'll be my own site's url). feel free to dm me @levlinds if you want to share your original personal ones with me, but it's not in my scope to navigate around next-auth in order to repro unfortunately :( hope you understand!

@zaarheed
Copy link
Author

zaarheed commented Feb 15, 2021

@lindsaylevine totally understandable. I've DM'd you with all the env vars you need to recreate the issue. Once you're done I'll delete them.

If anyone else needs access to the vars to investigate, feel free to DM at @zaarheed and I'm happy to provide.

Appreciate any insight and guidance on this issue :)

@lindsaylevine
Copy link
Contributor

lindsaylevine commented Feb 15, 2021

@zaarheed hello! so i believe this may actually be a next-auth issue after all... but not totally sure next-on-netlify is absolved of all responsibility quite yet. cc @balazsorban44

what i do know is that the root of the issue starts here: https://github.com/nextauthjs/next-auth/blob/82dd6ba3e4ba481c5003c918eb1d2fa95f7d76dd/src/adapters/typeorm/index.js#L101

in the lambda that Next.js generates for your API route /api/auth/next_auth (aka what we host in a netlify function), there's the require_optional call for 'mongodb', which eventually takes us to the line that's failing:

Screen Shot 2021-02-15 at 10 52 26 AM

Screen Shot 2021-02-15 at 10 52 53 AM

location = path.dirname(location);

id argue that the condition that allows this line to run is a bug (aka } else if (location !== '/') {). this condition will pass when location is undefined, causing path.dirname to error out with the error we're seeing.

that said, i acknowledge this code likely never expected location to be undefined. this is where the fault could be on netlify's side. i'll have to do a bit more digging there to know for sure, but i'm not sure right now why this require_optional logic is necessary. this bug persists even when mongodb is installed as a normal dependency in the project.

@zaarheed
Copy link
Author

@lindsaylevine Interesting, thanks for taking a look. It sounds very similar to this issue I saw on the next-auth repo earlier.

@lindsaylevine
Copy link
Contributor

if you include package.json in the list of files that get bundled and deployed and that should resolve the issue - I think there are some old issues (possibly closed now) that cover this.

interesting comment from @iaincollins ... i'm not sure this is a fair expectation? traditionally the package.json is excluded from bundles and publish dirs on purpose 🤔

@lindsaylevine lindsaylevine added priority: medium type: bug code to address defects in shipped code labels Feb 15, 2021
@lindsaylevine
Copy link
Contributor

lindsaylevine commented Feb 16, 2021

@zaarheed hey hey - i hate to send you on a wild goose chase. it's frustrating bouncing between repos, for sure. that said, i don't think there's anything left i can do on netlify's end. this is an issue specifically with next-auth and how next is bundling/generating this lambda representing /api/auth/[...nextauth]. when i log currentModule in the source at this point i get:

Screen Shot 2021-02-15 at 9 14 46 PM
Screen Shot 2021-02-15 at 9 15 06 PM

this of course means currentModule.filename is undefined, causing location to be undefined. the module value comes from the way this lambda is generated by next.js. i could possibly help you personally with a patch, unrelated to netlify, to unblock you, but i'm not sure what we can do on our end :/. if you wanted to temporarily use patch-package to move forward, let me know, and we can try to figure that out together. yk how to reach me on twitter as well!

if a next or next-auth maintainer can prove past what i've proven that this is in fact netlify specific, i'll be happy to dig deeper into their findings!!

@lindsaylevine lindsaylevine added the next-auth related to next-auth label Feb 25, 2021
@balazsorban44
Copy link

To say something here, I don't like require_optional, and would like to move to something like peerDependenciesMeta. We are also in the process of moving out the adapters into their own packages. (https://github.com/nextauthjs/adapters)

Related issue: nextauthjs/next-auth#1012

@brenelz
Copy link

brenelz commented Mar 9, 2021

So I think I am having this issue. Does next-auth work on netlify currently?

@zaarheed
Copy link
Author

zaarheed commented Mar 9, 2021

@brenelz not working for me when using Mongo as the database

@lindsaylevine has kindly shown me how to use patch-package to see if that will get me around the issue but I haven’t had a chance to try it yet

@lindsaylevine
Copy link
Contributor

@brenelz yeah, it doesn't work. i think next-auth needs to support serverless environments to be compatible with netlify (see: nextauthjs/next-auth#887 which @zaarheed shared earlier in the thread).

@balazsorban44
Copy link

balazsorban44 commented Mar 9, 2021

@brenelz not totally sure what you mean by us working on netlify? We try not to specifically code against any platform but we do support all the platforms that can run Next.js correctly.

@lindsaylevine we do support serverless, and it works fine for most of the people. (an example of our effort to support serverless is how we bundle css, see the comments in this file:
https://www.github.com/nextauthjs/next-auth/tree/main/src%2Fcss%2Findex.js)
If you are having issues, please report it under our repository with a complete reproduction, so we can track down the problem.

@zaarheed
Copy link
Author

zaarheed commented Mar 9, 2021

@balazsorban44 ticket already created with repro: nextauthjs/next-auth#1311

Feel free to DM me for the env vars

@lindsaylevine
Copy link
Contributor

@balazsorban44 i see two issues on next-auth for two different serverless environments where it doesn't work, closed because "it works for most people". as demonstrated earlier in this thread, there's nothing next-on-netlify can do to fix this issue.

@iaincollins
Copy link

iaincollins commented Mar 9, 2021

Hi there! Thanks @lindsaylevine for the research.

@brenelz yeah, it doesn't work. i think next-auth needs to support serverless environments to be compatible with netlify (see: nextauthjs/next-auth#887 which @zaarheed shared earlier in the thread).

NextAuth.js can be used on Vercel, AWS Lambda, AWS Lambda @ Edge as well as with containers (Heroku, Fargate, etc).

When folks run into problems it's usually because of how things are being bundled or deployed; there are several different ways of packaging up Node.js - and specifically Next.js applications - even within the Serverless framework itself.

Not all hosting platforms - or run time environments! - work with all Node.js modules on NPM and this is a common problem that trips folks up. Handling runtime dependancies in particular are a gotcha for both platform developers and users of those platforms. I think this is going to be a challenge for edge cases for a while yet.

e.g.

  • CloudFlare edge workers are JavaScript but is crucially not Node.js, so many NPM modules don't work.
  • Some environments (including Deno) don't support dynamic imports, though they are investigating how to address that.
  • Some serverless platforms non JS/JSON static assets that will be loaded at runtime to be explicitly included at build time (and, less commonly, some don't support this at all).

We try to accommodate platform specific issues where we can - such as by wrapping CSS in a JS shim because some bundlers ignore .CSS files and that was really common and easy to address - but there are limits to what we can reasonably do as a volunteer maintained project; we try to serve the greater good so have to make the case to leave resolving some problems up to other folks, especially when those problems are platform specific.

Why require_optional?

I've written quite extensively about this in the past but the tl;dr is we used to use dynamic imports - because that's what they are for - but it's even less well supported so require_optional, which is used by MongoDB itself, was chosen because we are using it to do the same thing the MongoDB driver does.

Some folks also run into issues with require_optional but crucially less folks than did with dynamic import (judging by the tickets we got and volumes of emails/DMs people sent me), so it was chosen as the least worst option available.

Ways to address this

We do have a longer term solution in mind for this but it's a work in progress effort.

Given people are not paying us, but are paying platform vendors, I'm include to push back a little and suggest that people ask their vendors to reconsider their implementations and note that problems like this are not going to be specific to NextAuth.js (e.g. I run into problems with packages that have i18n support as they tend to load language files dynamically).

I would note that NextAuth.js does work on other serverless platforms. There is working demo of it on Vercel prominently on the front page to illustrate this: https://next-auth-example.now.sh/

I understand the challenge for vendors but note that anything that involves a custom asset bundling solution means it will break expectations for modules assuming a typical Node.js run time environment - so vendors probably have a choice to make here about how they position their serverless solutions (this may come down to a cost / feature trade off).

Short Term Fixes

These are things we could do, but I am not inclined to do for the reasons stated (and because it works on other platforms):

  • Fork require_optional to fail more gracefully - extra work to maintain, may require multiple breaking changes in an attempt to fix in a way that works for everyone.
  • Refactor the code to import additional dependancies required under the hood directly - this would probably work, might be fragile and would increase bundle size for everyone.
  • Change our API for MongoDB so that the MongoDB adapter needs to be passed in directly - would work but would complicate the API and may increase our support overhead (which is time from volunteers and could almost be a full time job for one person already :-).

Long Term Fix

The long term plan is to split out next-auth into something like @next-auth/core and remove the adapter code.

Folks would then be able to load adapters from separate packages, like @next-auth/mongodb-adapter. The Prisma folks have been working on an improved Prisma adapter which I'm particularly excited about.

This approach would reduce bundle size for everyone, as you'd only need to load the adapter logic you needed, makes it easier to maintain different adapters (based on what usage they get) and avoids issues like this entirely.

This is probably where we can best spend our time and effort which is why I'm not really concerned about workarounds for specific platforms right now, however if someone does have a fairly simple suggestion (i.e. a very small and very clear change) they have tested which seems safe to implement for others we are very open to try rolling that out.

Workaround

You could always just copy the adapter code and put it in your own repo, and remove the offending dependancies and lines :-) If you want you could even re-share that adapter for others to use.

NB: I'm not sure how the MongoDB driver works on the same platform in the same context, given it also uses require_optional (which is the only reason NextAuth.js uses it) and both the official MongoDB driver and NextAuth.js are using the same version of this package.

https://github.com/mongodb/node-mongodb-native/search?q=require_optional

It might be worth confirming you can load the MongoDB driver itself from a Next.js API route in the same environment and check there isn't an underlying problem there that would prevent anything from working regardless.

@brenelz
Copy link

brenelz commented Mar 10, 2021

So I got things up a running with 2 different things needed. This patch-package thing is cool!

  1. My [...nextauth].js looks like the following:
export default (req, res) => {
  const { context } = req.netlifyFunctionParams || {};
  if (context) {
    context.callbackWaitsForEmptyEventLoop = false;
  }

  return NextAuth(req, res, options);
};

2, Used the patch-package to change var mongodb = (0, _require_optional.default)('mongodb'); to var mongodb = require('mongodb'); in node_modules/next-auth/dist/adapters/typeorm/index.js

@lindsaylevine
Copy link
Contributor

@iaincollins thank you so so so much for the detailed breakdown! that's super insightful and helpful. will keep this in mind with other next-auth-related issues that come netlify's way.

@brenelz excellent!!!! thank you for sharing this. that patch is what i suggested to @zaarheed in dms, though i definitely was missing the callbackWaitsForEmptyEventLoop piece. it doesn't work without that? how'd you know to do/try that?

@brenelz
Copy link

brenelz commented Mar 10, 2021

@lindsaylevine well the first issue was the netlify function was just hanging and timing out. Thats what the callbackWaitsForEmptyEventLoop is for. To be honest I found it by lots of googling and trial and error.

I seems like mongodb needs that for some reason. Some links that I found were:

https://docs.atlas.mongodb.com/best-practices-connecting-to-aws-lambda/
#66 (comment)

Been learning a lot about nextjs and netlify recently :)

@balazsorban44
Copy link

Happy that Iain could give an insightful answer! I am grateful for his experience/support and sorry that I could not help myself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
next-auth related to next-auth priority: medium type: bug code to address defects in shipped code
Projects
None yet
Development

No branches or pull requests

5 participants