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

Add helpful error for cyclic dependencies #280

Open
cllns opened this issue Dec 11, 2024 · 0 comments
Open

Add helpful error for cyclic dependencies #280

cllns opened this issue Dec 11, 2024 · 0 comments

Comments

@cllns
Copy link
Member

cllns commented Dec 11, 2024

If you have a component Foo that imports a Bar component, and that Bar component imports a Foo component... that's a cyclic dependency, which is bad. If you do that with dry-system, Ruby raises a SystemStackError. This is great because we don't want to allow cyclic dependencies.

However, we should provide a more helpful error message, to help users figure out how to fix it. With a simple cycle of 2 components, that's easy enough to write out ("Foo depends on Bar and Bar depends on Foo") but it become unclear when the cycle is any longer. Elm handles this by showing an ASCII (unicode?) art graph, and I'd be happy if we ended up with something similar.

These dependencies form a cycle:

Foo ───► Bar ───► Baz
▲                  │
│                  │
└──────────────────┘

You must fix this in order to use any of them.

Here's the current error message:

/Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/loader.rb:64:in `constant': stack level too deep (SystemStackError)
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/loader/autoloading.rb:17:in `require!'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/loader.rb:47:in `call'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/component.rb:64:in `instance'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/container.rb:639:in `block in load_local_component'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-core-1.0.2/lib/dry/core/container/item/callable.rb:16:in `call'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-core-1.0.2/lib/dry/core/container/resolver.rb:36:in `call'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-core-1.0.2/lib/dry/core/container/mixin.rb:132:in `resolve'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/dry-system-1.1.1/lib/dry/system/container.rb:493:in `resolve'
	 ... 10417 levels...
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/bundler-2.5.10/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
	from /Users/sean/.asdf/installs/ruby/3.3.6/lib/ruby/gems/3.3.0/gems/bundler-2.5.10/exe/bundle:20:in `<top (required)>'

I think we'll need to detect and prevent cycles before they attempt to get loaded, to preempt this error. This involves some algorithms that can be found in textbooks :)

Or, ideally we could even rescue the SystemStackError and go through the stack until we find a repeat and collect all the keys, so we could get this without any overhead in the system (instead of trying to preempt the SystemStackError)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant