You see this code in a review. What is wrong with it and how would you fix it?
answer
- No 'Caused by:' = cause was dropped
- Pass e as the second constructor arg
- Add super(message, cause) constructor
- initCause(e) if no cause constructor
- Translating type good; dropping cause bad
basics
~10 sIt throws a new exception but never passes the original (e) as the cause, so the real error and its stack trace are lost. Fix it by chaining: throw new ServiceException("...", e).
solid answer
~50 sThe bug is that the catch block translates the exception to a higher-level type but doesn't chain the original — it constructs ServiceException with only a message, dropping e. This is exception masking: when the failure surfaces, you'll see 'ServiceException: could not load user' with no 'Caused by:' section, so you lose the actual reason (e.g. a connection timeout) and the original stack trace, making the bug far harder to diagnose. The fix is to pass e as the cause: throw new ServiceException("could not load user " + id, e). That preserves the full chain in the stack trace while still exposing a layer-appropriate type to callers. If ServiceException lacks a (String, Throwable) constructor, add one that calls super(message, cause), or use initCause(e). Translating the type is fine and good; the only defect is dropping the cause.
code
java · 16 lines// BEFORE (bug): cause dropped -> no 'Caused by:' in trace
catch (DataAccessException e) {
throw new ServiceException("could not load user " + id);
}
// AFTER (fixed): cause chained -> full diagnostic chain preserved
catch (DataAccessException e) {
throw new ServiceException("could not load user " + id, e);
}
// Requires a cause-accepting constructor:
public class ServiceException extends RuntimeException {
public ServiceException(String message, Throwable cause) {
super(message, cause);
}
}go deeper
Spots that e isn't passed and fixes it by chaining; knows the (String, Throwable) constructor pattern.
Explains why the missing cause matters for debugging and that translating the type itself is correct.
Notes that logging-only is insufficient, discusses initCause vs constructor, and ties it to team lint rules forbidding masking.
Connects to org-wide error-handling standards, static-analysis enforcement, and how lost causes degrade incident response/observability.
## The code under review ```java public User loadUser(long id) { try { return repository.findById(id); } catch (DataAccessException e) { throw new ServiceException("could not load user " + id); // BUG: e is not passed } } ``` ## What is happening here (terms defined) An **exception** is an object signalling a failure that propagates up the call stack. A **stack trace** is the recorded list of method calls showing *where* it happened. The **cause** of an exception is another exception that triggered it; Java prints it as a `Caused by:` block in the trace, forming a *chain* back to the original error. This method does **exception translation** — it catches a lower-level `DataAccessException` and throws a higher-level `ServiceException` appropriate to the service layer. Translation itself is *good practice*: it stops the data layer's type from leaking to callers. ## The defect: the cause is dropped (masking) The `ServiceException` is created with only a message string. The caught exception `e` is **never attached**. This is **exception masking** (or swallowing the cause). Consequences: - The printed trace shows only `ServiceException: could not load user 42` and the stack from *this* method onward. - There is **no `Caused by:`** — the real reason (maybe a SQL timeout, a constraint violation, a closed connection) and its stack trace are gone. - Whoever debugs the production incident has lost the single most useful piece of evidence. ## The fix: chain the cause Pass the original exception as the **cause** so the chain is preserved: ```java public User loadUser(long id) { try { return repository.findById(id); } catch (DataAccessException e) { throw new ServiceException("could not load user " + id, e); // e chained as cause } } ``` For this to compile, `ServiceException` must have a constructor that accepts a cause: ```java public class ServiceException extends RuntimeException { public ServiceException(String message, Throwable cause) { super(message, cause); // passes the cause up to Throwable } } ``` If you can't add such a constructor, use `Throwable.initCause`: ```java ServiceException ex = new ServiceException("could not load user " + id); ex.initCause(e); throw ex; ``` Now the trace shows the `ServiceException` *and* `Caused by: ...DataAccessException...` with the original stack — full diagnostics retained, while callers still only see the clean `ServiceException` type. ## What is NOT wrong - **Translating the type is correct** — exposing `ServiceException` instead of `DataAccessException` keeps the data layer from leaking. - **Catching `DataAccessException` specifically** (rather than `Exception`) is fine and even good. The entire bug is the missing cause. The lesson: *translate the type, but always chain the original.*
- How do you preserve the cause if ServiceException has no (String, Throwable) constructor?Either add one that calls super(message, cause), or after constructing the exception call ex.initCause(e) before throwing it.
- Is just logging e and then throwing the new exception without the cause acceptable?No — logging helps locally but any upstream handler that catches ServiceException still sees no cause. Chain the cause so the information travels with the exception.
saying these in an interview costs you the question
- Saying the whole catch should be removed (translation is fine)
- Logging e but still throwing without it (partial fix; still masks for upstream handlers)
- Catching Exception instead of the specific type as the 'fix'
- Thinking the message string alone preserves the original error