Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal errors followup #1830

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Fatal errors followup #1830

merged 2 commits into from
Sep 5, 2024

Conversation

lovromazgon
Copy link
Member

Description

Followup to #1811 that changes a few things:

  • cerrors.FatalError only wraps the error into a fatal error if the supplied error is not nil and if it's not already a fatal error (this prevents multiple fatal error: fatal error: ... messages)
    • added tests for both cases
  • removed unnecessary check for in fatalError.Error, since now the Err field in fatalError is guaranteed to be populated
  • removed duplicated test
  • changed the DLQ node to only return a fatal error if the DLQ is actually enabled (i.e. if window nack threshold is greater than 0)
    • also removed the wrapped error message if the DLQ is not enabled
  • made sure the processor node also returns a fatal error if the processor doesn't return an expected type (e.g. if it returns a nil record)

Quick checks

  • I have followed the Code Guidelines.
  • There is no other pull request for the same update/change.
  • I have written unit tests.
  • I have made sure that the PR is of reasonable size and can be easily reviewed.

@lovromazgon lovromazgon requested a review from a team as a code owner September 4, 2024 17:59
@lovromazgon lovromazgon enabled auto-merge (squash) September 5, 2024 12:09
@lovromazgon lovromazgon merged commit 7e7bb8b into main Sep 5, 2024
3 checks passed
@lovromazgon lovromazgon deleted the lovro/fatal-errors branch September 5, 2024 12:14
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.

2 participants