Skip to content

feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern (MAPCO-9803)#17

Merged
almog8k merged 25 commits intomasterfrom
MAPCO-9803/implement-cleaner-worker-skeleton
Feb 22, 2026
Merged

feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern (MAPCO-9803)#17
almog8k merged 25 commits intomasterfrom
MAPCO-9803/implement-cleaner-worker-skeleton

Conversation

@almog8k
Copy link
Collaborator

@almog8k almog8k commented Feb 9, 2026

Question Answer
New feature
Documentation
Tests added

Further information:
Implements the foundational skeleton for the cleaner worker service, including error handling, validation infrastructure, and the strategy pattern for task execution.

Changes

  • Add error handling framework (RecoverableError/UnrecoverableError/ErrorHandler)
  • Add Zod-based validation infrastructure with TaskValidator
  • Add strategy pattern (ITaskStrategy, StrategyFactory, TilesDeletionStrategy stub)
  • Configure queue polling settings across all deployment levels
  • Update testing documentation with hybrid architecture
  • Remove boilerplate demo code (logistics)
  • Add comprehensive unit tests.
    What's NOT Included (Future Work)
  • TaskPoller implementation (orchestration layer)
  • Actual tiles deletion logic (stub only)
  • Full DI container wiring for cleaner services

- Add configuration sync across all deployment levels (default.json, env vars, Helm)
- Create complete custom-environment-variables.json with all config mappings
- Add helm/local.yaml for local development overrides
- Update Helm chart values and ConfigMap template
- Add config/local.json example for local development (gitignored)
- Document configuration sync requirement in common-patterns.md
- Update gitignore to exclude local.json
- Add implementation-plan.md to gitignore for local use only
- File can be regenerated as needed and doesn't need version control
- Add QUEUE_CLIENT, TASK_POLLER, STRATEGY_FACTORY, TASK_VALIDATOR, ERROR_HANDLER tokens
- Add migration note for jobnik-sdk tokens
- Document that strategies use task type strings instead of Symbol tokens
- Update common-patterns.md with correct Symbol syntax
- Move polling.dequeueIntervalMs to queue.dequeueIntervalMs
- Move polling.pairs to queue.pairs
- Update env var name: POLLING_DEQUEUE_INTERVAL_MS → QUEUE_DEQUEUE_INTERVAL_MS
- Simplifies config structure - all queue-related settings in one place
- Update all config levels: default.json, env vars, Helm values/templates
- Add RecoverableError for retryable failures (network, timeouts)
- Add UnrecoverableError for permanent failures (validation, config)
- Add ValidationError for schema validation failures
- Add StrategyNotFoundError for missing task strategies
- Add core type definitions: PollingPairConfig, QueueConfig,
  HttpRetryConfig, ErrorContext, ErrorDecision, TaskResponse
- Add createMockLogger() test helper for reusable logger mocking
- Add TaskValidator class for type-safe parameter validation
- Add taskSchemas map for registering task-specific schemas
- Add placeholder tilesDeletionParamsSchema (TODO implementation)
- Validation failures throw ValidationError (unrecoverable)
- Add basic test coverage for validation infrastructure
- Add zod@^3.25.76 to match version from mc-utils
- Required for validation schema definitions
- Ensures single version across dependencies
- Add ErrorHandler class to decide retry vs reject for task errors
- Handle RecoverableError: retry if attempts < maxAttempts
- Handle UnrecoverableError: never retry (validation, config errors)
- Handle unknown errors: treat as recoverable with retry fallback
- Log errors with full context (job, task, attempts, error details)
- Add comprehensive test coverage (100% for ErrorHandler)
- Replace generic examples with actual project patterns
- Add hybrid philosophy (unit *.spec.ts + integration *.integration.spec.ts)
- Document testing decision guide (when to use unit vs integration)
- Add real examples from codebase (ErrorHandler, createMockLogger)
- Include faker usage examples for test data generation
- Add troubleshooting section for common issues
- Document selective test running commands
- Remove outdated boilerplate references
- Keep guide concise and practical (~220 lines)
…n stub

- Add ITaskStrategy<T> interface with generic params for type-safe execution
- Strategy receives only validated params, no context duplication
- Add StrategyFactory using DI container to resolve strategies by task type
- Add TilesDeletionStrategy stub implementing ITaskStrategy<TilesDeletionParams>
- Factory resolves strategies from DI container via string tokens
- Factory throws StrategyNotFoundError for unregistered task types
- Add comprehensive unit tests for StrategyFactory (5 test cases)
- TilesDeletionStrategy implementation deferred (TODO with placeholder)
- Strategies focus on business logic, orchestration layer handles context
- Remove src/logistics/ directory (demo manager and types)
- Remove src/seeder.ts (demo job/task seeder)
- Remove tests/logistics.spec.ts (demo integration tests)
- Update containerConfig.ts to remove logistics types
- Update index.ts to remove seedData demo call
- Replace worker.ts with stub pending TaskPoller implementation
- Reduces test count from 16 to 14 (removed 2 demo tests)
@almog8k almog8k changed the title feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern(Mapco 9803) feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern(Mapco-9803) Feb 9, 2026
@almog8k almog8k changed the title feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern(Mapco-9803) feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern (Mapco-9803) Feb 9, 2026
@almog8k almog8k changed the title feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern (Mapco-9803) feat: implement cleaner worker skeleton with error handling, validation, and strategy pattern (MAPCO-9803) Feb 9, 2026
@github-actions
Copy link

github-actions bot commented Feb 9, 2026

🎫 Related Jira Issue: MAPCO-9803

Copy link

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

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

After our conversation, implement an abstract class of TaskStrategy that includes validate method, avoid "validation" dir by move schemas directly into the cleaner

Copy link

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

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

After our conversation, implement an abstract class of TaskStrategy that includes validate method, avoid "validation" dir by move schemas directly into the cleaner

Copy link

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

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

After our conversation, implement an abstract class of TaskStrategy that includes validate method, avoid "validation" dir by move schemas directly into the cleaner

Copy link

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

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

After our conversation, implement an abstract class of TaskStrategy that includes validate method, avoid "validation" dir by move schemas directly into the cleaner

Implement capability declaration pattern with explicit job/task pairs
instead of cartesian product. Worker declares specific job/task
combinations it handles, validated against global job definitions with
ConfigurationError for misconfigurations.
Move task validation from centralized TaskValidator into individual
strategies. Simplify StrategyFactory to single resolveWithContext()
method for automatic logger enrichment with complete job context.

- Add validate() method to ITaskStrategy interface
- Create validateSchema() helper using DI container for logger
- Implement validate() in TilesDeletionStrategy
- Simplify StrategyFactory: remove resolve(), keep only resolveWithContext()
- Add TaskContext with jobId, taskId, jobType, taskType for full tracing
- Remove TaskValidator class and taskSchemas registry
- Add comprehensive tests for validation and context enrichment
- Fix mock logger to include child() method
* Configuration error - invalid or missing configuration.
* This is always unrecoverable and indicates a deployment/configuration issue.
*/
export class ConfigurationError extends UnrecoverableError {

Choose a reason for hiding this comment

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

as we discussed, extends from Error & configuration schema will be implemented within mapcolonies schemas repo to reuse the validation of infra config

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will create the relevant Schema for the cleaner config. Will added to mapcolonies/schemas and will added to cleaner in different pr

Choose a reason for hiding this comment

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

no need for logger mock, we can just define Logger as {enabled: false} as we usually do,
exmaple:
`

errorHandler = new ErrorHandler(jsLogger({ enabled: false }));

Choose a reason for hiding this comment

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

furthermore i think its irrelevant to test the logger its self with toHaveBeenCalledTimes() or toHaveBeenCalledWith()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are using logger abilities like child logger per container so we need to make sure that the logger behave as expected- for this we must mock the logger instead of using:
jsLogger({ enabled: false })

Choose a reason for hiding this comment

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

I can relate to that, but as best practice we should test the act itself (that logger as been called) but not the exactly log message as it breaks frequently

refactor(validation): update logging messages in validateSchema function
refactor(tests): simplify ErrorHandler tests by removing mock logger assertions

Choose a reason for hiding this comment

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

I can relate to that, but as best practice we should test the act itself (that logger as been called) but not the exactly log message as it breaks frequently

Copy link

@CL-SHLOMIKONCHA CL-SHLOMIKONCHA left a comment

Choose a reason for hiding this comment

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

Good job, remember to delete local.yaml file after you ignore it from git

@almog8k almog8k merged commit bf43c75 into master Feb 22, 2026
6 checks passed
@almog8k almog8k deleted the MAPCO-9803/implement-cleaner-worker-skeleton branch February 22, 2026 13:33
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