Skip to content

feat: remove printStackTrace from exception handlers#84

Merged
nomeguy merged 2 commits intomasterfrom
copilot/remove-print-stacktrace
Nov 14, 2025
Merged

feat: remove printStackTrace from exception handlers#84
nomeguy merged 2 commits intomasterfrom
copilot/remove-print-stacktrace

Conversation

Copy link
Contributor

Copilot AI commented Nov 14, 2025

The codebase was printing stacktraces to System.err before re-throwing exceptions, implementing the "log and throw" anti-pattern. This forced stacktraces into console output even when callers intentionally ignored specific exceptions.

Changes

  • Removed e.printStackTrace() calls from 4 exception handlers in JDBCBaseAdapter.java:
    • savePolicy()
    • addPolicies()
    • removePolicies()
    • updatePolicy()

Before:

} catch (SQLException e) {
    conn.rollback();
    
    e.printStackTrace();  // Forces output to System.err
    throw e;
}

After:

} catch (SQLException e) {
    conn.rollback();
    throw e;  // Caller decides how to handle
}

Exceptions are still thrown and can be logged by callers at the appropriate level.

Original prompt

This section details on the original issue you should resolve

<issue_title>Do not print stacktrace</issue_title>
<issue_description>"Log and throw" is considered an anti-pattern, because an exception should not be handled multiple times and it can lead to duplicate log entries.
(Also printing directly to System.err without using a logging facade is not nice ...)

In my opinion this line (and 3 more like it in the same file) should be removed:

In my case, I want to ignore a certain exception - which works fine - but I still see the satcktrace in the output, which I would not like to see.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@hsluoyz
Copy link
Member

hsluoyz commented Nov 14, 2025

@tangyang9464 @imp2002 please review

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nomeguy nomeguy marked this pull request as ready for review November 14, 2025 13:23
Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove stacktrace printing from JDBCBaseAdapter Remove printStackTrace from exception handlers Nov 14, 2025
Copilot AI requested a review from nomeguy November 14, 2025 13:30
@nomeguy nomeguy closed this Nov 14, 2025
@nomeguy nomeguy reopened this Nov 14, 2025
@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.33%. Comparing base (490645a) to head (834c8dd).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #84   +/-   ##
=======================================
  Coverage   73.33%   73.33%           
=======================================
  Files           3        3           
  Lines          90       90           
  Branches       26       26           
=======================================
  Hits           66       66           
  Misses         13       13           
  Partials       11       11           
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nomeguy nomeguy changed the title Remove printStackTrace from exception handlers feat: remove printStackTrace from exception handlers Nov 14, 2025
@nomeguy nomeguy merged commit 4b13e08 into master Nov 14, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not print stacktrace

4 participants

Comments