Core: Env.getOrFail method proposal

[moltar]

Brief Description

Sometimes a certain environment variable is essential for running the app. It'd be good to be able to assert presence while getting the value.

This would greatly aid debugging of cross-environment apps. And prevent developers from forgetting to set env in production.

Sample Code

Env.getOrFail('FOO')

Would throw something like that, if no value is provided:

throw InvalidArgumentException.missingParameter('Env.get', 'FOO')

[thetutlage]

I agree! But do you think that this may not work seamless when you have the same call inside a Controller, since controller are executed when someone visits a related route.

Maybe we can have some sort of validation file that runs before the server starts and dies immediately.
In that file you can write Env.exists('FOO') and not only environment checks, you can even drop functions to check for other stuff too

[moltar]

You have a good point re run-time use. Did not consider that.

I try not to refer to Env inside controllers, or really anywhere outside of config files. I think it's a bad practice to litter the code with Env stuff. And because config files are loaded right away, the app would crash.

I think defining required env variable in another file is a good workaround, but would still be prone to errors.

Maybe just documenting the fact that this feature should only be used in a config file would be enough?

[thetutlage]

Yes agree with that too. I believe we can have a note in the Env docs, that it is recommended to use the Env provider inside the config files only and use the config elsewhere in your app.

Can you please create PR's for same? I will be happy to accept them

[webdevian]

I also have a rule that Env should only be used in config files, to make sure they all get resolved at runtime. I think this is a good idea

[moltar]

I will work on it next week.

[moltar]

What should the new method name be?

I proposed getOrFail, which is inline with the Lucid naming (e.g. findOrFail).

You proposed to use exists.

To me, exists appears misleading, because it makes it sound like it'd check for existence rather than return a value.

[thetutlage]

Yes getOrFail speaks for itself.

[webdevian]

I like getOrFail

[moltar]

I am wondering if we might need a new generic exception method for this? I repurposed another one a bit, but it does seem hack-ish.

Do we maybe want:

throw RuntimeException.missingEnv('FOO')

Thoughts?

[thetutlage]

Yes, indeed we should have a specific exception code for missing env values.

message, 500, 'E_MISSING_ENV_KEY'

[moltar]

Being blocked by release of https://github.com/adonisjs/adonis-generic-exceptions/pull/3

[thetutlage]

Released as 2.0.1

[moltar]

@RomainLanz Why did you close this issue?

[moltar]

I'm getting excessive diff when doing npm install in the package-lock.json file. I've tried running npm with and without --save-exact and always get the same result. Any ideas?

-        "node-exceptions": "3.0.0",
-        "upcast": "2.1.2"
+        "node-exceptions": "^3.0.0",
+        "upcast": "^2.1.1"

-        "@adonisjs/generic-exceptions": "2.0.0",
-        "co-compose": "4.0.0",
-        "debug": "3.1.0",
-        "haye": "2.0.1",
-        "lodash": "4.17.5"
+        "@adonisjs/generic-exceptions": "^2.0.0",
+        "co-compose": "^4.0.0",
+        "debug": "^3.1.0",
+        "haye": "^2.0.1",
+        "lodash": "^4.17.5"

And many other similar changes.

[thetutlage]

That doesn't make much difference.

[moltar]

I think there is a difference. One says "install this exact version", and the other says "install any version equal or above this one".

I thought the point of the lock file was to lock EXACT versions to make the environment reproducible.

Beyond that there are security implications. There is no guarantee that malicious code will not be introduced in a minor or patch change. E.g. if someone had vetted all of the dependencies and they want to install the exact ones, confirmed by hash, then they should be able to do so.

[thetutlage]

I understand one says to use the exact and other allows, minor and patch releases.

There is no guarantee anyways and this is where common-sense and selectively choosing packages comes into the picture. I only use dependencies from known authors and ofcourse, it not possible for me to look at each package or author as malicious or think they will turn into evil one day.

[moltar]

Maybe it is not possible for you. But I know organizations that vet everything. Even if just automatically. If you want projection adoption, you have to play nice with the community :)

I am just trying to understand why this even happens. If we can easily avoid this, I don't see a reason not to. I think exact versions are better than loose versions.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.