Giter Site home page Giter Site logo

Comments (39)

crisu83 avatar crisu83 commented on May 1, 2024 7

@GrahamCampbell The behavior is indeed correct in Lumen. However if you decide to use a .env file during development and run something else in production with the current implementation you will need to add unnecessary logic for this in your bootstrap file.

This is not how the other dotenv libraries works. The ruby implementation ignores the error by default (https://github.com/bkeepers/dotenv/blob/master/lib/dotenv.rb#L14), while the node implementation just logs the error by default (https://github.com/motdotla/dotenv/blob/master/lib/main.js#L39), and there is even a option to make it silent.

If you port a module from one programming language to another, I think that the implementation should be as close to the original implementation as possible. That makes it easier to work with the library if you already worked with another dotenv library in another programming language.

@vlucas Can we re-open this issue and implement it in a future version?

from phpdotenv.

tuupola avatar tuupola commented on May 1, 2024 6

You could include empty .env file?

from phpdotenv.

alexweissman avatar alexweissman commented on May 1, 2024 6

@vlucas if the try/catch method is the preferred pattern for dealing with alternative environment configurations, it might be worth documenting this in the README.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024 5

@GrahamCampbell I still don't like the fact that this library works differently than the original Ruby library. Also, that try / catch block with an empty catch doesn't look too nice either.

from phpdotenv.

vlucas avatar vlucas commented on May 1, 2024 2

This behavior cannot be changed at this point. Too many people rely on this behavior for it to be changed now. Just wrap your call in a try/catch.

from phpdotenv.

gaguirre avatar gaguirre commented on May 1, 2024 1

We use environment variables in part because of the 3rd factor.
Adding specific code for production environment (the try/catch block in this case) goes against that point in some way.

I'm with @crisu83, it should work like the original library.

from phpdotenv.

jartaud avatar jartaud commented on May 1, 2024 1

@msheakoski Sorry! i had a silly error:

if (file_exists($envFile));
{
    (new Dotenv\Dotenv(dirname($envFile)))->load();
}

see the semi-colon?

from phpdotenv.

jartaud avatar jartaud commented on May 1, 2024 1

@crisu83 Yeah! I've updated the comment. Thanks!

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024 1

@markushausammann We now have two methods you can call, and the exception types are well-defined, so you can know what to catch if you want the hard fail. We have load and safeLoad: https://github.com/vlucas/phpdotenv/blob/v3.3.3/src/Dotenv.php#L71-L98.

If the file doesn't exist, then InvalidPathException will be thrown only by load. Both load and safeLoad will throw InvalidFileException if the file has a syntax error, however.

from phpdotenv.

apotek avatar apotek commented on May 1, 2024 1

Sometimes people use frameworks and aren't in control of how their framework use phpdotenv. In our CI tooling, we constantly get these red hot errors in our logs:

    ERROR: file_get_contents(/app/.env): Failed to open stream: No such file or directory 

    in /app/vendor/vlucas/phpdotenv/src/Store/File/Reader.php:73

To me, a missing .env should trigger a notice or a warning (at most). Not all environments in your pipeline use .env files and not all projects can control the way their framework implements .env support. So we are stuck scrolling through error messages that should be notices, or modify framework source code, which is not a good idea.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

I could, but it's not an optimal solution because I don't want to create or keep around an empty .env file that I'd include in my build. Adding an empty .env file sounds like a hack.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

That's no better than checking if the file actually exists. If the project would use proper versioning (semver) you could change this for the next major release without worrying to break BC.

from phpdotenv.

KIVagant avatar KIVagant commented on May 1, 2024

Please, add configuration attribute for this behaviour. I totally agree with @crisu83, this exception does not need on production environments. Now, all times, I need to create empty .env file in project root.

from phpdotenv.

vlucas avatar vlucas commented on May 1, 2024

You don't need to deploy an empty .env file - just do a file_exists check before using Dotenv instead.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

@vlucas I think that's unnecessary boilerplate, if the library automatically loads a file it should ensure that the file actually exists before trying to load it.

from phpdotenv.

vlucas avatar vlucas commented on May 1, 2024

@crisu83 It does, which is why it throws an Exception if it does not exist. You can also wrap the Dotenv call in a try/catch block as well.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

Of course you can, but it feels unnecessary to add such logic to your bootstrap file. Here's how dotenv is used by Lumen, Laravel's micro framework https://github.com/laravel/lumen/blob/master/bootstrap/app.php#L5. Maybe that gives you a better idea of what I'm after here.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

I think this behviour is correct. See how it's commented out. That's what you do if you don't want to load a .env file.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

@vlucas I still think that we should re-open this issue, just because it works differently in the PHP implementation than in the other implementations.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

We have changed the exception we throw. You can now catch it. There's no real issue here.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

See how we catch the exception specifically for if the file is not found: https://github.com/laravel/framework/blob/v5.2.32/src/Illuminate/Foundation/Bootstrap/DetectEnvironment.php#L22-L26.

from phpdotenv.

nnnnathann avatar nnnnathann commented on May 1, 2024

Just thought I would throw my 2 cents in for not throwing on file not found... I realize it is an easy fix on the user end but catches me every time in production :)

Either way, thanks for the superb library!

from phpdotenv.

msheakoski avatar msheakoski commented on May 1, 2024

I know this has been closed for a while, but for future readers:

The most performant solution would be file_exists() because as they say, no code is faster than no code. This solution does not load the Dotenv code into memory and does not execute any further instructions in the PHP interpreter. That's what you want for production, the most optimized path.

$envFile = "path/to/.env";
if (file_exists($envFile)) {
    (new Dotenv\Dotenv(dirname($envFile)))->load();
}

Having Dotenv ignore the missing file internally still loads Dotenv into memory, and instantiates objects such as Loader and InvalidPathException. A try/catch block, like one that @GrahamCampbell pointed to in Laravel, would also do the same with the added overhead of the instructions for handling the exception.

from phpdotenv.

jartaud avatar jartaud commented on May 1, 2024

@msheakoski even with file_exists returning false, I'm still getting: Uncaught Dotenv\Exception\InvalidPathException: Unable to read the environment file at

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

@jartaud you mean semi-colon?

from phpdotenv.

markushausammann avatar markushausammann commented on May 1, 2024

I agree, it should not throw an exception at all. A lot of implementations use .env files only on development platforms while staging and production, etc. use a different implementation of environment variables.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

@markushausammann An example of using safeLoad is available at https://github.com/laravel/lumen-framework/blob/v5.8.8/src/Bootstrap/LoadEnvironmentVariables.php if you are interested. It is called at https://github.com/laravel/lumen/blob/v5.8.0/bootstrap/app.php#L1-L7.

from phpdotenv.

crisu83 avatar crisu83 commented on May 1, 2024

@GrahamCampbell While it's good that this issue finally got mended after four years, I'm still a bit unsatisfied with the solution as this differs from the other implementations. 😐

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

I don't agree that it does. Neither load nor loadSafe is the default behaviour. It's up to the caller to choose which they want. Thus, it does not differ in any meaningful way, only naming.

from phpdotenv.

mykytak avatar mykytak commented on May 1, 2024

@GrahamCampbell Some silentLoad method would be perfect, with same try-catch under the hood, don't you think? Especially if you have default values in your project that could be altered by .env file (so .env file is optional). In that case try-catch feels like unnecessary complication.

And with load method, try-catch block is a part of usage but I don't see any mention about that in readme.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

@mykytak safeLoad does exactly what you say already.

from phpdotenv.

mykytak avatar mykytak commented on May 1, 2024

@GrahamCampbell safeLoad throws file_get_contents(...): failed to open stream: No such file or directory exception. Yes, it is not fatal but still exception. So you need to wrap it anyway.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

Does it? I can't replicate that?

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

This has to be a bug in your custom error handler. We suppress the error.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

https://github.com/vlucas/phpdotenv/blob/v3.6.0/src/Loader.php#L111-L150

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

Make sue your custom error handler looks aterror_reporting(). If it's 0, ignore the error - it was suppressed by the code, and not meant be handled.

from phpdotenv.

mykytak avatar mykytak commented on May 1, 2024

@GrahamCampbell it's CodeIgniter default I think. Maybe it was altered in some way, don't know.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

Well, something in your code is grabbing up that error, and incorrectly propagating it. Specifically, something in your registered error handlers. Are you seeing your app crash, or just it being logged?

If you write a single PHP file and just load composer's autoload file and then the phpdotenv code, that should confirm there's no bug in the code here.

from phpdotenv.

GrahamCampbell avatar GrahamCampbell commented on May 1, 2024

Hiding the above comment because the author has not read the README or any of this thread. Going to lock this now, since safeLoad is already implemented, and available.

from phpdotenv.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.