cider-jack-in: Add -XX:-OmitStackTraceInFastThrow by default

[timvisher]

Expands cider-clojure-cli-jack-in-dependencies to also add :jvm-opts ["-XX:-OmitStackTraceInFastThrow"] in the :cider/nrepl alias so that
there are less situations where CIDER encounters a stack trace that has no
traceback.

May be worth extending to the other cider-*-jack-in-dependencies
functions.

Fixes #3022

[bbatsov]

I think it'd be weird if we only harcoded this for people using clj. Ideally the defaults should be consistent across all build tools.

[dpsutton]

I think the only thing to do here is gracefully handle empty stack traces. We should not add jvm arguments. CIDER already injects quite a bit in order to run, requiring jvm args seems too far. Especially if people are already specifying this behavior. Not sure if both args is an error or one silently overrides the other.

Seems far preferable to just add some kind of nil check on the stacktrace and show what information is available, if any.

[timvisher]

@dpsutton That seems like a good idea. @bbatsov Are you down with switching to that direction?

[timvisher]

@dpsutton I will say that an argument for adding this as a default argument is that I can't think of many times when I would want my stack traces omitted in dev. It feels like a better default for a development environment.

Interestingly, lein appears to set this by default already.

Maybe that points to the idea that clj is actually where this should be set by default rather than patched in via CIDER?

[dpsutton]

I will say that an argument for adding this as a default argument is that I can't think of many times when I would want my stack traces omitted in dev.

Exceptions in hot paths will pay a penalty. Core.match uses this for backtracking I believe. I understand the gist of what you are saying but I disagree. The reason I think this is a bad default is that you've already shown where some tooling has a default. Having several layers to check for this (lein, CIDER) makes it harder to get the behavior you want. I'd far prefer someone setting this flag (if desired) in the standard way and then no tooling mucking with it and confusing them why it has been overriden, or startup throws an error because the arg for ommitting stacktraces has been included twice, or they said don't create stacktraces and CIDER includes an option to create stacktraces.

I'm a big fan of CIDER helping out on top of existing tooling. If lein sets this, then that sets the expectation and its documentation will explain why this is set, and people have to live with their choices. I cannot imagine clj will add this by default. But adding another layer where options are set sounds more complicated than necessary.

[kxygk]

I just happened to come across this issue b/c I got fed up with seeing NullPointerException with no stacks :) - thank you for pointing out the issue.

I'd think CIDER should at least notice that your NullPointerException has no stack and suggest the potentially reason (and how to fix it). I've had these empty NullPointerException errors pop up for months and frankly my initial reaction was that it was probably some bug with CIDER (or my emacs init file) So I'd never even tried investigating it till now. I had no idea this was an "optimization" . Again, the way it's handled at the moment kinda just makes CIDER look buggy in my opinion

Hope this is resolved somehow :) I still get empty NullPointerException sometimes, but less..

[timvisher]

@bbatsov Are you down with switching to that direction?

@bbatsov Sorry if I'm being too pushy here. Just a quick ping. :)

[bbatsov]

Yeah, I'm totally find with adding some handling of those empty stack frames in CIDER. I assume that should be pretty empty to do.

[timvisher]

I assume that should be pretty empty to do.

Was this a bit of freudian sarcasm? xD

I'll see if I have some time to dig into this. Is there any chance you could point me at some likely spots to handle this as well as a rough description of that way you'd like to see it implemented?

Initially my instinct is to have it as some kind of message directly in the stack trace buffer with a description of what could be done to turn the fast trace behavior off.

[bbatsov]

Was this a bit of freudian sarcasm? xD

Just an epic typo. 😆

Is there any chance you could point me at some likely spots to handle this as well as a rough description of that way you'd like to see it implemented?

I don't remember the code well, but I assume this should be handled somewhere in/near cider-stacktrace-render-frame. I guess we can just display some message in the minibuffer or something along those lines.

[bbatsov]

Any progress here? I'm just wondering how to proceed here.

Maybe that points to the idea that clj is actually where this should be set by default rather than patched in via CIDER?

I guess that's up to @puredanger & co, but it sounds like a reasonable thing to do, if Lein has been doing it for a while. For the same reason I'm not opposed to just hardcoding it in CIDER, as the current patch suggests.

I've also updated CIDER's docs to mention this issue, so it's easier to discover if people run into it.

[timvisher]

Any progress here?

Sorry I haven't made any time to address this. I'd still like to but responsibilities have taken me elsewhere. :(

I've also updated CIDER's docs to mention this issue, so it's easier to discover if people run into it.

That's great! :)

For the same reason I'm not opposed to just hardcoding it in CIDER

That's good to know. I still think that figuring out a way to handle the empty stack traces in CIDER is the best option and I'd like to explore that but I'm also not opposed to this patch being merged and then adding that later.

[bbatsov]

Superseded by #3042.