Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Log the framework-provided exceptions with no stacktraces #159

Merged

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Aug 1, 2019

Do not print stacktraces of internal temporary/permanent errors, just log the message.

Issue : #16

Description

Shorter logging

The internal temporary and permanent errors are special exceptions to control the framework's flow of handler execution: either to retry it, or to stop trying — regardless of the global mode on arbitrary error handling (in the assumption that retry_on_error will be exposed to the users some day).

They are supposed to already contain the information needed to identify the cause of the problem. Stacktraces are not needed (they are needed only for the unexpected errors with unclear origin).

Instead, stacktraces of these special errors make it difficult to identify when an actual unexpected errors happens (by visually or automatically looking for stacktraces in the logs).

Therefore, the stacktraces are removed for these special error — and only for them. All unexpected arbitrary exceptions still have stacktraces as before.

Exceptions renamed

As a little refactoring, the exception classes are renamed for clarify, and to remove the mentions of "handler" from their names:

  • kopf.HandlerRetryError --> kopf.TemporaryError
  • kopf.HandlerFatalError --> kopf.PermanentError

The old names are kept as the aliases (so they still can be used as kopf.HandlerRetryError/kopf.HandlerFatalError in try-except clauses or any other places).

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor/improvements

Review

  • Tests
  • Documentation

@nolar nolar added the enhancement New feature or request label Aug 1, 2019
@nolar nolar requested a review from samurang87 as a code owner August 1, 2019 19:39
@zincr
Copy link

zincr bot commented Aug 1, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Aug 1, 2019

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@codecov-io
Copy link

codecov-io commented Aug 1, 2019

Codecov Report

Merging #159 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   82.76%   82.82%   +0.05%     
==========================================
  Files          32       32              
  Lines        1596     1601       +5     
  Branches      231      232       +1     
==========================================
+ Hits         1321     1326       +5     
  Misses        230      230              
  Partials       45       45
Impacted Files Coverage Δ
kopf/reactor/handling.py 85.85% <100%> (+0.21%) ⬆️
kopf/__init__.py 100% <100%> (ø) ⬆️

@nolar nolar changed the title Log framework-provided exceptions with no stacktraces Log the framework-provided exceptions with no stacktraces Aug 1, 2019
@nolar nolar merged commit d057ac9 into zalando-incubator:master Aug 7, 2019
@nolar nolar deleted the no-stacktraces-on-special-errors branch August 7, 2019 17:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants