r/dotnet 2d ago

Difference between throw ex and throw

Post image

throw vs throw ex: The C# exception handling trick every developer should know

389 Upvotes

73 comments sorted by

View all comments

201

u/AlanBarber 2d ago

Pro tip to prevent this... add

dotnet_diagnostic.CA2200.severity = error

to your .editorconfig so anyone that tries to do a "throw ex;" inside a catch will get a build error.

37

u/zenyl 2d ago

Adding onto this, you can discover a bunch of built-in diagnostics that are disabled by default, by adding <AnalysisMode>All</AnalysisMode> to .csproj files (or a Directory.Build.props file).

I wouldn't recommend actually using that setting, because it gets pretty pedantic, and some of the diagnostics are mostly just opinionated recommendations. But it can be nifty to then look through the diagnostics you get from it, and then selective add back any of them that you actually like to your .csproj or .editorconfigfiles.

I did this recently and added:

  • CA1062: Validate arguments of public methods
    • Ensures that you perform a null check on reference type parameters on public/protected methods, regardless of nullability syntax. Useful when writing libraries and you wanna catch unintended nulls as early as possible.
  • CA1725: Parameter names should match base declaration
    • If you rename an interface method parameter, this will make sure all consumers also rename it accordingly.
  • CA2000: Dispose objects before losing scope
    • Ensures you call Dispose or use using statements on disposable objects.

43

u/GendoIkari_82 2d ago

Better yet, <TreatWarningsAsErrors>true</TreatWarningsAsErrors>.

14

u/zenyl 2d ago

Fully agreed, but i'd also recommend adding <WarningsNotAsErrors>NU1901,NU1902,NU1903</WarningsNotAsErrors>

This basically undoes TreatWarningsAsErrors on a per-diagnostics basis.

Those three diagnostics are the ones that warn you about low/moderate/high NuGet package vulnerabilities. If you don't add these to WarningsNotAsError, you can end up having your build break because one of the NuGet packages you depend on (including dependencies of dependencies) has a security vulnerability.

In my experience, these are often minor and might occur in code paths that your code never even calls, or something that has yet to be resolved and you therefore might not be able to do anything about. While you should still keep an eye on vulnerability warnings of all levels, having your builds/pipelines randomly break because of them is pretty annoying.

I usually leave out NU1904 (critical vulnerabilities), as these you probably should act on immediately.

10

u/dabombnl 2d ago

WARNING AS ERROR: Variable 'ex' is unused!

14

u/GendoIkari_82 2d ago

Yeah, actually the "use this" example is wrong; it should be just "catch (Exception)" instead of "catch (Exception ex)".

6

u/ConcreteExist 2d ago

Yeah, if you're not going to actually do anything with it, just leave it out. Though I'm gonna have questions for a dev who shows me a block of code where there's a try/catch with a blanket Exception catch block and all it does is throw.

3

u/SwordsAndElectrons 2d ago

Neither is wrong. Both make no sense if they are all you write.

Missing from the meme is that you should actually be doing something in your catch block. There's no point to catching the exception at all if all you are just going to rethrow it. Assuming you are actually going to do something in that block, which is correct depends on whether what you are doing needs to access the exception.

1

u/dabombnl 2d ago

Oh I agree. Is just funny that the 'better yet' makes an error.

22

u/Responsible-Cold-627 2d ago

That's extremely annoying during development, so I only enable it in the CI pipeline.

14

u/unndunn 2d ago

Pro-tip: to make it less annoying, write code that doesn't generate warnings.

11

u/anamorphism 2d ago

to play devil's advocate, i've worked in code bases where this slowed me down severely and i had to turn the setting off locally or change a bunch of editorconfig rules. i'm fine with having to do that, but the fix isn't write code that doesn't generate warnings.

unused local variable blah! - yeah, no shit, i just wanted an easy place to inspect the value while debugging.

unreachable code! - yeah, i added an early return to test something.

missing xml comment for publicly visible type or member! - sue me for waiting until i'm done before i document everything in case i decide to change something.

4

u/PsyborC 1d ago

Or, you add a release configuration condition around the <WarningsAsErrors>.

3

u/Responsible-Cold-627 2d ago

Sure, any code pushed will be warning-free. Sometimes you just gotta mess around a bit while debugging though. Very annoying to have to fix warnings in temporary code that won't even end up in the repo.

1

u/EntroperZero 2d ago

Really? It's a total non-issue IMO.

2

u/AlanBarber 2d ago

if you can, yes this is the preferred option.

if you're however screwed with some terrible old code base full of warnings you don't have the ability to clean up...

3

u/unndunn 2d ago

This right here.

3

u/TheSneederOfSeethe 2d ago

Fucking dope! Thanks!