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

Don't fail when there's a loop in symlinks #839

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Y--
Copy link
Contributor

@Y-- Y-- commented Mar 8, 2016

No description provided.

@3y3
Copy link
Member

3y3 commented Mar 13, 2016

Thank you for contribution.
Can you please add test here?

@Y--
Copy link
Contributor Author

Y-- commented Mar 13, 2016

Hi @3y3, thanks for considering my PR.
Sorry for the delay, I've finally got time to add a test.
It is not ideal but I could not find a better way to check whether an error occurred here or not :

if (error) console.warn('Access to ' + node + ' from ResourceTree denied. (' + error + ')');

Let me know if you have any suggesting to improve it.

Thanks!

@3y3
Copy link
Member

3y3 commented Mar 14, 2016

This is not important to check errors here.
You need only check that you have expected result with correct input.

@Y--
Copy link
Contributor Author

Y-- commented Mar 14, 2016

The expected result is tested.
Checking the errors that way is the only way I found to ensure that the algorithm won't follow the symbolic links until it faces an "ELOOP" error.
I can modify the implementation to have it being tested properly though
On my environment, I simply cannot use the current version of node-inspector because of this behavior, since it takes several minutes to start (or crash).

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

Successfully merging this pull request may close these issues.

None yet

2 participants