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

Fix error not being reported properly #396

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jessie-ross
Copy link

This PR fixes a gap in errors being reported.
Related to #256

I haven't looked at the whole code context so I am not sure if this is the best way to do so - let me know.

Replicating the issue:

  1. Create new deps.edn project.
  2. Add a file with a dash in it's name instead of an underscore (which every beginner is going to do 😭 ).
  3. Try to evaluate something in that file's context.
  4. See the same error as in Every clojure exception shows E716: Key not present in Dictionary #256

**This PR fixes a gap in errors being reported.**
Related to tpope#256 

I haven't looked so closely so I am not sure if this is the best way to do so.

Replicating the issue itself:
1. Create new deps.edn project.
2. Add a file with a dash in it's name instead of an underscore.
3. Try to evaluate something in that file's context.
4. See the same error as in tpope#256
@tpope
Copy link
Owner

tpope commented Dec 6, 2021

Can you share the full error output? E716 is pretty generic; I wouldn't jump to the conclusion that it's the same root cause as that 5 year old issue.

state.err in this context is stderr. Its presence does not necessarily warrant an exception.

@jessie-ross
Copy link
Author

Can you share the full error output?

Sure!

Screen Shot 2021-12-07 at 12 18 38 pm

With the error occuring on this line.

And this is what I see with the fix:

Screen Shot 2021-12-07 at 12 17 18 pm

@jessie-ross
Copy link
Author

state.err in this context is stderr. Its presence does not necessarily warrant an exception.

I am checking msg.err here, not sure if that is different, also msg.ex is set to an exception class if we want to branch on that.

@tpope
Copy link
Owner

tpope commented Dec 7, 2021

I meant to say msg.err.

What are the full contents of msg.err? You can use let err = 'Clojure: ' . json_encode(msg) to check.

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.

2 participants