crash if trying to set path to /

[mifi]

fixes #4271

[github-actions]

Diff output files

No diff

[Murderlon]

Why don't we allow this? I'd rather make this work as expected than let it crash.

[mifi]

This is just a quick fix, because if the user tries to set path to / it causes all kinds of hard to debug issues. I agree that rewriting all the code to accept a slash would be preferable but it needs deep changes in multiple files, changes which are hard to test/verify. I think it's a high risk change that could cause regressions so I don't think it's worth the risk for now. With this PR, at least it crashes early so that it's easy for the developer to know what's wrong, which IMO is much better than without it.

[Murderlon]

I wonder how much effort that is, because I think it's the better improvement and doing this just sounds like we're never going to get back to it. It's very common for people to setup routers to "/" and I don't think that behaviour should be punished.

What do you think? Worth the effort?

[mifi]

i don't think the risk is worth the benefit of allowing /. maybe we can add a TODO about allowing it in the future, that way it doesn't sound like we're never going to get back to it?

[movy]

Folks, just to clarify from user's perspective: omitting path var is equivalent to setting up router on the root path. The issue is hidden somewhere where path is applied to the root locaiton, and if it's set to / it produces // which (surprisingly) throws whole routing in disarray. If path is unset, companion works from root location just fine.

[mifi]

Yes so i think this pr solves you problem, which is wasted time debugging, right?

[movy]

@mifi , the error message you added states 'server.path cannot be set to /' which is not what actually happens. Path is set to / when variable is unset.
I think a better message would be If you want to use '/' as server.path, leave 'path' variable unset.

[mifi]

#5010