DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Method to wrap an exception in a RuntimeException. #1697

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently Throwables.propagate(e) throws an exception and also returns it (but 
actually never returns it because one is thrown).

This is quite bad if you want to control what exactly comes out of a method, 
for instance by logging it.

With log4j2, you have the very, very nice flow tracing methods (entry, exit, 
catching and throwing).

So look at this code (using an external cache tool, but you can read without 
knowing the api):

String getFromCache(String key) {
  logger.entry(key);
  Future<Object> f = this.cache.asyncGet(key);
  try {
    return logger.exit((String)g.get(500, TimeUnit.MILLISECONDS));
  } catch (TimeoutException e) {
    f.cancel(false);
    throw logger.throwing(Throwables.propagate(e));
  } catch (Exception e) {
    throw logger.throwing(Throwables.propagate(e));
  }
}

So what will this method perform when an exception occurs?
1. Log the entry (as expected)
2. Never log the result since an exception occurred (as expected)
3. Never log the exception being thrown. (not as expected)

Why do I log "throwing" an exception and not "catching" one as the flow tracing 
allows me? Because this comes from a third party library. I cannot be sure 100% 
that the exception is logged from the third party. So in this context, my 
method assumes the role the first "thrower" even though it's not.

Oh! A simple fix would be to wrap the logger.throwing around the exception 
before propagating it instead of after. Well, no. Because then my logs wouldn't 
be accurate anymore because they'd show an exception that is indeed correct but 
that is not the one my method actually throws (that is a RuntimeException).

So please provide some extra methods that simply wrap the exception instead of 
wrapping it then throwing it.

Then what to do about Errors? Well, I don't know. I usually let them go through 
all the stacks and I catch them as late as possible. So I don't really mind 
about what they do.

Original issue reported on code.google.com by ogregoire on 14 Mar 2014 at 11:20

GoogleCodeExporter commented 9 years ago
We tend to view "log and rethrow" as an antipattern: Either handle the 
exception, or throw. If you throw, use exception chaining. That way, the 
eventual handler has all the context in one place rather than split across 
multiple log messages.

But I'm not sure I've ever heard someone make a case for "log and rethrow." 
Want to give it a shot?

Original comment by cpov...@google.com on 14 Mar 2014 at 2:07

GoogleCodeExporter commented 9 years ago
I understand that you should either act (log) or rethrow. And I completely 
agree with that.

However, we're not in an all-pinky world and not all the exception handling is 
perfect in a project and if later on my exception is swallowed by some code I 
didn't write and yet we have an issue, I still want to be able to trace where 
it originated from. Using the flow tracing helps doing that.

Actually, my logger is configured to print the full stack trace only in WARN 
and higher. (also, "throwing" has been reduced from ERROR to TRACE)

So for tracing purposes, I should be able to see when my execution enters a 
method and when it leaves it. Using propagate() bypasses my tracing.

Original comment by ogregoire on 14 Mar 2014 at 2:43

GoogleCodeExporter commented 9 years ago
I would suggest a custom method:

throw wrapAndLog(logger, e);

It's less code, and it saves us from worrying about whether the inclusion of a 
wrap() method will encourage misuse.

Original comment by cpov...@google.com on 14 Mar 2014 at 3:36

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:09

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07