- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Awww... How cute.
Admin
So ... TRWTF is that spelling "ocurred" when every illiterate numpty calling themselves "programmer" just knows it's really spelt "occured"?
Admin
So sometimes two wrongs do make a right.
Also interesting choice of languages, nice mix of Spanish, (almost) good English and bad English. But I guess that's the new normal -- my previous TV set displayed some messages in German (my choice), some in English (alright, maybe no translation available), a few in French (hmm) and one in Finnish (but I guess that was their idea of a pun, since it was the message about automatically shutting down after some time, so a "finish" message) ...
Admin
Wait, isn't the rethrow command just "throw;" instead of "throw ex"?
The second will destroy the stack trace, IIRC.
Admin
TRWTF is that broken P tag at the beginning ;)
Admin
Don't
catch
unless you can do something useful with the exception. That code… doesn't do anything useful at all. Also, don't log if you're re-raising the exception; you do not want exceptions to be logged multiple times and they're noisy enough without that additional burden.Admin
While I agree, I want to add, that you do not HAVE to log the stack trace, especially if you exactly know what is wrong and where. For example when you want to open a file and catch a FileNotFound, then PLEASE do not just log the exception including stacktrace, which might not even include the filename. Instead log what the problem was, the context (what the program was trying to do) and what the program will do in response to that problem. So something like: could not open file xyz.txt while attempting to count words; assuming empty file and using word count 0
Admin
If bad code fails to do anything in a forest, does it exist?
Admin
I wonder if this is a case of two mistakes (not to use a heavier word) cancelling each other out, or a clever programmer forced to use a mandated wrong approach, introducing a subtle bug to do the right thing while seemingly following the rules...
Admin
Is this an artefact of something having been ported up from an earlier language? Seems odd someone knows enough about exception handlers to write this but not realise it is, at best, surplus to requirements.
Mind you, that probably covers a lot of the better WTFs on this site.
Admin
Yes,
throw ex
will remove the existing stack trace from the exception and create a new one from the current line. There are probably times when that is useful, but it's generally not the intended effect. In fact, intentionally usingthrow ex
to erase the stack trace is so unusual that I would make sure to leave a comment clarifying that it's not a mistake.As you said, the correct way to rethrow is to just use
throw
alone. Or if you need to modify the exception in some way (change its message, change its type, etc.), create a new exception and pass the original into its constructor, preserving the original (including its stack trace) as an inner exception.Addendum 2021-04-12 12:17: Edit: The above assumes C#, which appears to be the language in use here.
Admin
When the gun somebody pointed at their own foot, jams.
Admin
I don't how dumb this is without some additional context. In the case of any multi-user system I would argue that in general you should not let an exception bubble up to the UI later where the exception message itself let alone a stack trace might be 'handled' by displaying the message to the user. It could very well create a security problem. Unless you absolutely know what any code you are calling into might do in terms of the information it puts in there. A SQL message like "cannot convert 'Bob Taft' to type BIGINT" could very well represent an confidentiality problem.
So catching exceptions logging the data to an appropriate secure location and raising a new exception where you the developer control the content of the message and you can be pretty confident either the stack trace won't be reported or it won't reveal any implementation details that should not be public, might be perfectly appropriate. That said including some unique id to correlate what you are letting the higher layers, remote consumers of your API etc see with the originally logged exception would be powerfully good idea to facilitate debugging
Admin
I still have PTSD from dealing with AD forests.
Admin
Assuming this is Java, this is perfectly fine. The stacktrace is stored inside the exception object. Don't know about other languages though
Admin
Reads the first code block
Oh, my... this looks like something I might have written back in the day. Has it finally happened? Has my own code been featured on TDWTF at last? Will the world now know my sins? Will I have to forever live with a mark of shame indelibly etched into the very framework of reality, the Internet?
Continues reading
Phew, not my code after all.
Admin
Perhaps in some languages, but not in e.g. Java.
There, the stack trace is assigned to/stored in an an exception when it is instantiated, not when it is thrown. Technically it is perfectly fine to instantiate an exception somewhere and then throw it from arbitrary other places. The resulting stack traces will be identical and simply point to the original spot of instantiation.
Admin
Only no·thing can have no properties, so if "it" has the property of being bad, it must exist.
Admin
So, the poor guys don't exist...
Admin
There is nothing right about this code. Simply removing this monstrosity of an exception handler will make the code much easier to maintain. Everyone above who mentioned that "throw ex" will destroy the stack trace is spot on and is a much bigger wtf that the useless method call.
Admin
I'm suspecting that this is a case of a junior dev coming up with a 'clever' idea for turning exceptions into something that can be displayed to the end user. Then a more senior dev 'fixes' it by throwing the original exception instead of the butchered one, but doesn't have time to check for other side effects of that function call. Nor does he have the inclination to club junior dev with a clue bat. So then junior dev copies the now fixed code to everything else that he writes - thinking that it is still creating exceptions that the user will be able to read.
Admin
This ExceptionManager sounds like certain kinds of micromanagers who like to stick their fingers into everyone's business as if they know what they're talking about, but while everyone listens intently to what they have to say when they're saying it, everyone just ignores what they said because they really have no idea what's going on and their suggestions are worthless.
Admin
Maybe I'm missing something but handle exception doesn't actually throw a new exception, it returns one. Which is then unused in the catch block.... which then simply re throws the original exception. Nothing is being eaten it's just unnecessary and does nothing.
Admin
Yes, you appear to be missing the last few paragraphs of the article, which point this out.
Admin
If this is c# (possibly other languages too) the
throw ex;
will replace the stack from the original exception with the stack from that throw ... so you STILL have a huge WTF and lose valuable stack info. That should bethrow;
to preserve stack.Admin
It's rather C#. The Java convention for method names is
lowerCamelCase
, it'sUpperCamelCase
for C#.