Skip to content

chore(history): populate errorType for incidents#1031

Merged
joaquinfelici merged 5 commits intomainfrom
432-IncidentType
Feb 26, 2026
Merged

chore(history): populate errorType for incidents#1031
joaquinfelici merged 5 commits intomainfrom
432-IncidentType

Conversation

@joaquinfelici
Copy link
Contributor

Rel to #432

Pull Request Template

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Test-only changes (no production code changes)

Testing Checklist

Black-Box Testing Requirements

  • Tests follow black-box testing approach: verify behavior through observable outputs (logs, C8 API queries, real-world skip scenarios)
  • Tests DO NOT access implementation details (DbClient, IdKeyMapper, ..impl.. packages except logging constants)
  • Architecture tests pass (ArchitectureTest validates these rules)

Test Coverage

  • Added tests for new functionality
  • Updated tests for modified functionality
  • All tests pass locally

Architecture Compliance

Run architecture tests to ensure compliance:

mvn test -Dtest=ArchitectureTest

If architecture tests fail, refactor your tests to use:

  • LogCapturer for log assertions
  • camundaClient.new*SearchRequest() for C8 queries
  • Real-World skip scenarios (e.g., migrate children without parents)

Documentation

  • Updated TESTING_GUIDELINES.md if adding new test patterns
  • Added Javadoc comments for public APIs
  • Updated README if user-facing changes

Checklist

  • Code follows project style guidelines
  • Self-reviewed the code
  • Added comments for complex logic
  • No new compiler warnings
  • Dependent changes have been merged

Related Issues

@yanavasileva yanavasileva requested review from HeleneW-dot and removed request for yanavasileva February 26, 2026 08:17
Copy link
Contributor

@HeleneW-dot HeleneW-dot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, just requesting change for one question regarding whether we could differentiate further for the no retries incident type 🚀

String type = c7Incident.getIncidentType();
String msg = c7Incident.getIncidentMessage();

if (msg == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the message also be an empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's an empty string, I think it means it was set like that on purpose, so I think it makes sense to return UNKNOWN rather than falling back to the incident type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, meant that we can catch this here too to return early with unkown 👍

if (msg == null) {
// fall back to job‑retries semantics
if (Incident.FAILED_JOB_HANDLER_TYPE.equals(type) || Incident.EXTERNAL_TASK_HANDLER_TYPE.equals(type)) {
return JOB_NO_RETRIES;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: could we differentiate between EXECUTION_LISTENER_NO_RETRIES and TASK_LISTENER_NO_RETRIES` here? Or doe they not map like that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, both map to JOB_NO_RETRIES.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think I read it the wrong way around, all good then

@joaquinfelici joaquinfelici merged commit 91ef95e into main Feb 26, 2026
27 checks passed
@joaquinfelici joaquinfelici deleted the 432-IncidentType branch February 26, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants