require does not cache exceptions

[stelcheck]

• Version: 8.0.0
• Platform: mac
• Subsystem: module

Ref: https://gist.github.com/stelcheck/0d981e2b951b8c95a3487a733fde1925

If upon require, a module file throws, it will be re-required again upon subsequent attempts. This means that the content of the file will be re-executed as well, which I believe is not likely to be a desirable behavior (otherwise one would manually clean up the require cache).

What I would suggest is instead to cache the thrown Error and re-throw it upon subsequent require attempts.

[tniessen]

No strong opinion, but I am slightly against this change for some reasons:

First, and most importantly, the vast majority of require() calls does not happen in try blocks. Most apps just crash if a module fails to load. If your code needs to include logic to handle exceptions thrown by require(), you might want to add logic to cache the result yourself, but modules should barely ever throw a recoverable Error while being loaded.

What is your use case? If you want to handle the case that a module does not exist, you can use require.resolve() to check without actually require()-ing the module.

the content of the file will be re-executed as well, which I believe is not likely to be a desirable behavior

I don't believe this is true for most users. When attempting to require() a second time, a condition might have changed which prevented require()-ing the first time, so users might expect the second require to succeed without having the clear the cache.

---

The documentation is a little vague about this point:

Multiple calls to require('foo') may not cause the module code to be executed multiple times.

If we decide not to do the suggested change, we might want to clarify this behavior in the documentation.

[stelcheck]

First, and most importantly, the vast majority of require() calls does not happen in try blocks. Most apps just crash if a module fails to load.

This is very true, and what I suggest would not change that behavior; if anything, it would make that behavior more consistent for all possible cases.

If your code needs to include logic to handle exceptions thrown by require(), you might want to add logic to cache the result yourself, but modules should barely ever throw a recoverable Error while being loaded.

require already has its own cache; it seems to me this is where the caching should occur.

Whether modules should throw an Error like that in the first place - recoverable or not - is of course debatable. However, the issue is that if a module does throw in such a way, then it may lead to very cryptic issues because of the fact that the code is executed more than once - when, as you pointed out, the documentation indicates that this should not be happening.

What is your use-case? If you want to handle the case that a module does not exist, you can use require.resolve() to check without actually require()-ing the module.

This is not what I am trying to do, as per the code sample I have attached to this issue.

I personally do not have a use case for that; what happened is that I am using a proprietary framework which includes a code path similar to what is presented here, and I ended up seeing the module code being executed twice.

In other words, I submit that providing a behavior similar to what I have suggested would make debugging much easier for cases where some dependency might be doing funky stuff like that.

I don't believe this is true for most users. When attempting to require() a second time, a condition might have changed which prevented require()-ing the first time, so users might expect the second require to succeed without having the clear the cache.

I find this rather unlikely (nor safe), but supposing there would be such use-case; in the case of trying to re-require code which is already in the require cache, you would explicitly have to clear the require cache before requiring once again. I do not believe having this requirement for required libraries threw an error upon require would be either wrong or architecturally problematic.

[refack]

This reminds me of "RFC 2308 - Negative Caching of DNS Queries (DNS NCACHE)" especially when considering ESM and import...
/cc @bmeck @ljharb @TimothyGu

[bmeck]

I wont be recommending any change of this magnitude to CJS; the ESM cache does properly save exceptions. The CJS ESM bridge was intentionally vague here, but recent change to ECMAScript spec mandates rethrowing and we can rephrase to be more concrete.

[tniessen]

@refack Negative DNS caches can be a PITA though...

[refack]

@refack Negative DNS caches can be a PITA though...

I know that's why I think it's a good comparison. Theoretically a failed require could succeed later (process.chdir(), target file created / fetched in runtime, storage can back online, etc.)

[stelcheck]

@refack chdir should not matter for how the cache is processed (the cache resolves a module/file to its absolute path and read/write from/to cache using this absolute path AFAIK).

As for the file system going offline/online, I guess that's a valid case. But I also think it is likely to be less common. In my opinion, it would make more sense to force cache invalidation if you want to reimport.

[refack]

@refack chdir should not matter for how the cache is processed (the cache resolves a module/file to its absolute path and read/write from/to cache using this absolute path AFAIK).

AFAIK require('./module') resolves based on pwd.

But anyway a neg-cache is a trade off, and it would be introducing a change to a very delicate mechanism.
I'm what we call -0 on this (objecting in a not blocking way), mostly because of the fragility of the CJS resolver 🤷‍♂️.

[stelcheck]

AFAIK require('./module') resolves based on pwd.

What I meant './module' will be converted to absolute path before being looked up in require.cache, but I had misunderstood your point (being that changing directory means relative requires no longer point to the same place, so the call may succeed). Sorry about that.

I wont be recommending any change of this magnitude to CJS; the ESM cache does properly save
exceptions. The CJS ESM bridge was intentionally vague here, but recent change to ECMAScript spec
mandates rethrowing and we can rephrase to be more concrete.

@bmeck does this mean require will rely on the import cache in the future? Or that it is already the case?

[TimothyGu]

@stelcheck

does this mean require will rely on the import cache in the future? Or that it is already the case?

No it is not, and will not. CJS and ESM are two different systems. I think Bradley was just pointing out the fact that ESM will have a different behavior.

[bnoordhuis]

It's been a while and there seems to be no consensus so I'll go ahead and close this out.