chore: ignore errors in telemetry handlers#1058
chore: ignore errors in telemetry handlers#1058crowecawcaw merged 3 commits intoaws-deadline:mainlinefrom
Conversation
github-actions
Bot
added
the
waiting-on-maintainers
| def is_initialized(self) -> bool: | ||
| return self._initialized | ||
|
|
||
| @_swallow_exceptions |
There was a problem hiding this comment.
Is a function like this really need the swallow exception decorator?
There was a problem hiding this comment.
I would think only the record functions should have ignore exceptions.
There was a problem hiding this comment.
It looks safe enough to me without it. Instead of looking at each method for risks, I just applied it everywhere that it wouldn't break something for completeness.
I can scale this back and just apply it to the heavier, top-level methods.
karthikbekalp
removed
the
waiting-on-maintainers
|
Hmm, looks this PR needs to be rebased as there a few merge conflicts. |
crowecawcaw
force-pushed
the
telemetry-error-handling-decorator
branch
2 times, most recently
from
fbfa483 to
17b2ba7
Compare
Narrow _swallow_exceptions to root telemetry entry points (set_opt_out, initialize, record_event, _exit_cleanly) instead of every internal helper. Protect __init__ with targeted try/except for _get_telemetry_identifier and _get_system_metadata. Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
crowecawcaw
force-pushed
the
telemetry-error-handling-decorator
branch
from
17b2ba7 to
9594ebd
Compare
|
1 similar comment
|
3 participants
Fixes: #755
What was the problem/requirement? (What/Why)
Telemetry handlers could fail causing the library/CLI to not work correctly. Telemetry is never required to function though.\
What was the solution? (How)
Ignore any telemetry errors so it never breaks the main application.
What is the impact of this change?
Better resilience in the event of telemetry issues.
How was this change tested?
Unit tests
Was this change documented?
n/a
Does this PR introduce new dependencies?
No
Is this a breaking change?
No
Does this change impact security?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.