Throw error when modules circularly require modules that are being required

When a module has been required, if you require it from another script before it is returned then the other script will yield until it has been returned to the first require call. This can cause issues if you end up making a module require a module that requires itself and can be a very easy to do in passing but not very easy to debug.

I would like to request that in this instance that an error be thrown when the second module attempts to require the first as it’s only going to yield indefinitely otherwise. This would make debugging much easier.

You can reproduce this behavior with two module scripts and one script.

-- Script source
require(script.Module1)
print("This won't print")

-- Module 1
local module = {}
require(script.Module2)
return module

-- Module 2
local module = {}
require(script.Parent)
return module
10 Likes

I don’t think such behavior should error. When things like this happen, it’s best to set break points and continue to modulize your code

2 Likes

Could always do something like

-- module code
local module = {}
function module.init()
require(script.Parent)
end
return module

--script
local m = require(script.Module1)
m.init()

With this method, it will finish the entire module, and then require should work.

Yes, that’s a solution to it. The problem is that if you do this without realizing it can be very confusing. If it is going to cause the thread to yield and never finish running then it seems like a good idea to throw a warning or error.

I’ve had this problem for a project of mine.
I have a moduleloader module and a lot of child modules.
Some of the child modules require each other, which is tricky.

The trick is to load the “linear dependant” modules first.
If module A doesn’t need anything, B needs A and C needs B, load (A,B,)C first.

For the cyclic modules, I just use something like:

-- Data module
-- load = require(script.Parent) -- moduleloader
local Network
spawn(function()
	Network = load("Network")
end)
-- Network module
-- load = require(script.Parent) -- moduleloader
local Data = load("Network")

Of course you could just in your code use load(“Network”):Send() instead of Network:Send().
It’s less efficient and uglier, but it probably works in lots of cases you don’t auto-run stuff immediatly.

@Brad_Sharp
An advantage of having a ModuleLoader is the fact you can monitor when a module gets loaded.
(And more importantly, if it ever finishes loading. Easy to debug with print(debug.traceback()) too)

They could spend time working this in script analysis, however it shouldn’t do anything at runtime

Yeah, this bug is around for a long time already. In default Lua you get an error. In roblox, however, you don’t get an error. Your script just get’s stuck. It doesn’t crash, it doesn’t error, you don’t even notice it…

1 Like

Why not? If this ever happens, something is obviously, inarguably wrong, and so it should be reported as clearly as possible at the source. Bear in mind that crashing the threads involved won’t change anything else, because those threads are deadlocked anyway!

2 Likes

Should WaitForChild or any other yield function stop abruptly?

Wait, why is this an issue? Logically speaking, this makes a lot of sense. If two modules require each other, you’d get stuck in an infinite loop. It makes sense to throw an error (or otherwise hit a stackoverflow error).

Requiring a module on ROBLOX isn’t the same as inclusions in compiled languages. When you require a module, it actually executes the code of the module (unless previously required before). Therefore, you have an infinite loop of the modules requiring each other since neither ever completes its execution fully.

4 Likes

Actually, this is what I assumed would happen. However, it’s not. If you call require on a module that has already been required but hasn’t returned anything yet (because it’s requiring the other module for example) then the second require will yield until the first require returns. This is where the issue lies, the first can never return and this the thread just reaches a dead end, rather than causing a stack overflow. It’s more like waiting for an event that is certain to never happen.

It doesn’t error and that’s wrong as this makes debugging extremely hard. If you have a codebase of a huge project in a compiled language I think it’s easy to make such mistake. I’m pretty sure compilers will throw an error. This is how it works in Lua 5.1. (cat is a command to show the contents of a file)

1 Like

I think he was replying to GollyGreg asking why throwing an error was an issue – not why the current behavior is an issue, because based on what he posted it sounds like he wants it to throw an error.

A custom require can fix all the issues.

Of course official support might be handy.

Bad einstienK. This thread’s existence is proof enough that it’s not “might”. It will be handy. Under no circumstance is not throwing an error for circular requiring helpful – it only makes it harder to debug. Throwing an error for circular requiring is better behavior in every case and should be the default.

If we decided which features were useful entirely based on that, we wouldn’t have seen: scrolling frames, GUI buttons, VIP servers, private servers, pathfinding, etc. Bad einsteinK.

2 Likes

[size=10]pls no butcher mah name[/size]

Can’t script VIP/private servers without kicking players on join.
(but I like the other (scriptable) things to be added though)

This is a tiny bit smaller compared to what you mentioned.
It’s more like the scripter didn’t think properly about module dependency.

That’s the entire reason human-readable errors exist. In the place of nothing happening / “something went wrong!” (yeah I figured), you have a helpful error that tells you why your code isn’t working which can save you hours of debugging time and adding print statements every other line. This certainly applies to circular requiring.

CreatePlaceAsync – what people were doing prior to the introduction of both of those features.

2 Likes

Right, that too.
But those aren’t really servers, they’re difficult to update.
(It’s possible to completely automate updating them, but it’s a pain)

I want to bring this back up again. When you are coding complex systems and making rapid changes, especially when working with multiple people across several features in a game, sometimes you accidentally introduce a circular require. It would be useful if a warning could be put in output that signals the user of this deadlock, so that they are immediately aware of what is going on and can start debugging / working around for that particular issue. Not having any kind of message or warning leads to additional effort needed to figure out where/when exactly a circular require has taken place.

12 Likes

Kind of weird that this is an unpopular/controversial thread, but this is one of the hardest bugs to debug, namely because

  1. It is silent
  2. Random systems fail unexpectedly, and it takes a while before you realize this is probably a circular dependency issue nested somewhere deep beyond the game’s entry points
  3. To make it not silent, you have to spam an ungodly number of print statements in positions that probably aren’t actually where things are going wrong
  4. These print statements are hard to clean up.

People make custom require-like higher order functions keep track of which modules have returned or are still in the process of being required without return, and error manually. For example, roblox-ts does this, and before they added it, I even modded the roblox-ts runtime library myself so that I would receive errors when circular dependencies.

Note that this is not saying that two modules should never require each other (though this is generally bad practice); rather, it is saying they should not require each other before they have returned, which causes this untraceable and painful-to-debug error.

I think this is a really easy and incontrovertible fix. Modules break at runtime when they require each other before returning. They might as well error as well. I’ve run into this issue so many times because of some accidental require statement in the code, and every time it’s been annoying to fix.

7 Likes