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

Prisma loads .env, processes differently from RWJS #5657

Closed
joconor opened this issue May 27, 2022 · 7 comments · Fixed by #6199
Closed

Prisma loads .env, processes differently from RWJS #5657

joconor opened this issue May 27, 2022 · 7 comments · Fixed by #6199
Labels
bug/confirmed We have confirmed this is a bug topic/config Babel, Webpack, ESLint, Prettier, etc.

Comments

@joconor
Copy link
Contributor

joconor commented May 27, 2022

Running yarn rw dev invokes

const rwProjectRoot = requireFromCli('./dist/lib/index.js').getPaths().base in packages/core/src/bins/redwood.ts

which invokes

import {
getPaths as getRedwoodPaths,
getConfig as getRedwoodConfig,
} from '@redwoodjs/internal'
in packages/cli/src/lib/index.js

which invokes

export { generate } from './generate/generate' in packages/internal/src/index.ts

which invokes

import { generateGraphQLSchema } from ‘./graphqlSchema' in packages/internal/src/generate/generate.ts

which invokes

import { rootSchema } from '@redwoodjs/graphql-server' in packages/internal/src/generate/graphqlSchema.ts

which invokes

export * from './functions/graphql' in packages/graphql-server/src/index.ts

which invokes

import { RedwoodError } from '@redwoodjs/api' in packages/graphql-server/src/functions/graphql.ts

which invokes

export * from './validations/validations' in packages/api/src/index.ts

which invokes

import { PrismaClient } from '@prisma/client' in packages/api/src/validations/validations.ts

And causes Prisma to attempt to evaluate .env. Usually this is successful. When Prisma generates client code, .prisma/client/index.js is generated with a config object. The config has relativeEnvPaths.rootEnvPath which normally points to the root of the RedwoodJS project, which is where it (usually) finds the .env file. But under some circumstances, the generated relativeEnvPaths.rootEnvPath may be null, in which case Prisma will not find and evaluate .env

Because Prisma is evaluating .env very early in the invocation of yarn rw dev, RedwoodJS's use of dotenv-default becomes a moot point. By default, dotenv-default (and dotenv) will not overwrite variables already present in the environment.

Normally, all this is fine. But it can lead to a subtle and difficult to diagnose bug: In those cases where Prisma fails to find & evaluate .env, and RedwoodJS itself does the evaluation of .env, RedwoodJS doesn't (seem) to handle multiline environment variables correctly. Prisma is masking this failure of RedwoodJS's parsing of .env.

My specific case of RedwoodJS not handling a multiline env var is with Google API credentials. This is a multiline single-quoted string with embedded double-quotes. A multiline JSON string.

CREDS='{
  "type": "sometype",
  "project_id": "someprojectid",
  "private_key_id": "aprivatekeyid",
  "private_key": "aprivatekey",
  "client_email": "clientemail",
  "client_id": "clientid",
  "auth_uri": "someuri",
  "token_uri": "thetokenuri",
  auth_provider_x509_cert_url": "thecerturl"
}'

When Prisma fails to find and evaluate .env, and RedwoodJS does, only the first line of that multiline variable is read (i.e., CREDS='{).

Suggestion: Prisma .env should be kept separate from RedwoodJS .env.

Second suggestion: RedwoodJS should provide the ability to pass options to dotenv/dotenv-default to allow control over how it parses multiline variables as well as other options available in that package.

@jtoar jtoar moved this to Triage in Main May 27, 2022
@jtoar jtoar added this to Main May 27, 2022
@simoncrypta simoncrypta added topic/config Babel, Webpack, ESLint, Prettier, etc. bug/confirmed We have confirmed this is a bug labels May 30, 2022
@simoncrypta simoncrypta removed the status in Main Jun 8, 2022
@simoncrypta simoncrypta moved this to Backlog in Main Jun 8, 2022
@jtoar
Copy link
Contributor

jtoar commented Jun 11, 2022

Hey @joconor, amazing breakdown! 👏

It definitely feels like there's starting to be a bit too much cruft in the code that runs when importing some Redwood packages... as you've demonstrated, it's quite the rabbit hole. And it also just feels a bit too mutative to have packages start executing code as soon as they're imported. I investigated it a bit here where I traced an error with the cwd option for the CLI: #5623.

At first as I was unsure how importing PrismaClient loads .env, but I now see that the getPrismaClient function in @prisma/client/runtime/index.js does indeed load env.

I'll try to find some time this week to dive into this more and consider the two suggestions you mentioned.

@dthyresson
Copy link
Contributor

@jtoar In Office Hours @joconor inquired about he status of this issue and how enviers are being used. Can we add this to the Triage and discussion for next status?

@jtoar
Copy link
Contributor

jtoar commented Jul 28, 2022

@dthyresson sure, and I can probably prioritize it now that the we're refactoring the CLI—I can make it a part of that.

@jtoar
Copy link
Contributor

jtoar commented Aug 11, 2022

I've been looking at this recently and while I think I'm starting to understand what's going on, some parts are still a little hard to put into focus 100%. But TL;DR I will fix this in steps so that RW always evaluates .env first.

As to parsing multiline env vars, we can easily add support for that by toggling this option: https://github.com/motdotla/dotenv/tree/v14.3.2#multiline. I'm linking to v14.3.2 because that's the version of dotenv dotenv-defaults is pinned to unfortunately. dotenv is already on v16, so it'd be ideal if they upgraded soon than later. Maybe I can spur them on.

For most commands, it's true that prisma evaluates .env first. But I'm not so sure that the execution order of it all is so simple as to render dotenv-defaults moot. I may be wrong cause there's a lot going on here and I'm not super familiar with prisma's internals, but I'll try to explain. It seems that the earliest env evaluation is in node_modules/.prisma/client/index.js as you said:

// node_modules/.prisma/client/index.js

// ...

const { warnEnvConflicts } = require('@prisma/client/runtime/index')

warnEnvConflicts({
    rootEnvPath: config.relativeEnvPaths.rootEnvPath && path.resolve(dirname, config.relativeEnvPaths.rootEnvPath),
    schemaEnvPath: config.relativeEnvPaths.schemaEnvPath && path.resolve(dirname, config.relativeEnvPaths.schemaEnvPath)
})
const PrismaClient = getPrismaClient(config)
exports.PrismaClient = PrismaClient
Object.assign(exports, Prisma)

It's warnEnvConflicts that's doing the .env evaluation here, without dotenv-defaults having run at all. But this is just to warn. That's still important obviously, but PrismaClient doesn't see .env until it's been instantiated: https://github.com/prisma/prisma/blob/65f49576640309f5806182fbc985161f25bc7c0d/packages/client/src/runtime/getPrismaClient.ts#L376

It evalutes .env again, just without checking for conflicts. And we don't instantiate PrismaClient till a function's actually executed.

When I turn on debug flags and step through code, I often see this:

  prisma:tryLoadEnv  Environment variables loaded from /Users/dom/prjcts/rwprj/rwprj/.env +0ms
  rw, loading env... # I added this for debugging
  prisma:tryLoadEnv  Environment variables loaded from /Users/dom/prjcts/rwprj/rwprj/.env +0ms
    [dotenv][DEBUG] "SESSION_SECRET" is already defined in `process.env` and was NOT overwritten
    [dotenv][DEBUG] "DATABASE_URL" is already defined in `process.env` and was NOT overwritten

Prisma is evaluating .env twice, sandwiching redwood's evaluation. So it seems that defaults still get loaded for env vars that are undefined. It's certainly unclean that prisma goes first, ever, so I'll fix that. We definitely want this to be deterministic.

I'm having a hard time getting the config option you mentioned to be null, but I don't doubt that that would be problematic.

And as to these suggestions

Prisma .env should be kept separate from RedwoodJS .env.

RedwoodJS should provide the ability to pass options to dotenv/dotenv-default to allow control over how it parses multiline variables as well as other options available in that package

For full transparency, right now I don't see either happening just yet. You can already pass options to dotenv via DOTENV_CONFIG_ env vars (see https://github.com/motdotla/dotenv#preload), although I acknowledge that could be inconvenient. But we're happy to bake in options like multiline that seem totally reasonable.

@redwoodjs-bot redwoodjs-bot bot removed this from Main Dec 7, 2022
@redwoodjs-bot redwoodjs-bot bot added this to Main Dec 7, 2022
@redwoodjs-bot redwoodjs-bot bot removed this from Main Dec 7, 2022
@redwoodjs-bot redwoodjs-bot bot added this to Main Dec 7, 2022
@redwoodjs-bot redwoodjs-bot bot removed this from Main Dec 7, 2022
@jtoar
Copy link
Contributor

jtoar commented May 5, 2023

@joconor sorry I didn't close this one out but it was fixed by #6347 and released in v3.6.0. If you're still having other issues I'm happy to take a look again

@jtoar jtoar closed this as completed May 5, 2023
@joconor
Copy link
Contributor Author

joconor commented May 6, 2023

@jtoar Thanks! I'll check it out in my environment. I remember it was a bit tricky to reproduce 😏

@jtoar
Copy link
Contributor

jtoar commented May 10, 2023

Those are the best kinds of issues 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/config Babel, Webpack, ESLint, Prettier, etc.
Projects
None yet
4 participants