Skip to navigation

Postal Service Evaluation

Table of Contents

  1. Executive Summary
  2. Architecture Overview
  3. Key Findings
  4. Dependency and Layer Analysis
  5. Testability Assessment

1. Executive Summary

The Postal Service is a message-driven microservice responsible for delivering platform messages to registered client applications via HTTP webhooks. The service subscribes to Azure Service Bus topics, attempts delivery to client endpoints, and implements exponential backoff retry logic for failed deliveries.

Strengths

  • Clear service responsibility: The service has a well-defined, focused purpose
  • Message-driven design: Properly uses Azure Service Bus for decoupled message processing
  • TypeScript strict mode enabled: Ensures strong type safety
  • Good test structure: Separation of unit, contract, and service tests with TEST_SCOPE filtering
  • CLA DevCloud2 framework integration: Proper use of framework-nodejs-* packages for logging, configuration, and web support
  • Graceful shutdown handling: Proper SIGINT handling for receiver cleanup

Critical Concerns

  • Awilix dependency unused: Both awilix and @fastify/awilix are listed as dependencies but are not used in production code (only in test scaffolding)
  • In-memory scheduling store: The createSchedulingStore() creates module-level in-memory state that will be lost on pod restart
  • HTTP is secondary to messaging: Fastify routes are minimal (only a test endpoint), yet the architecture appears structured for HTTP-primary services
  • Global module-level caching: Multiple global Map instances (cached.ts, cachedForKey.ts) lack invalidation and could cause memory issues

2. Architecture Overview

2.1 Technology Baseline

ComponentTechnologyVersion
RuntimeNode.js20.x (Docker image)
LanguageTypeScript5.9.3
Module SystemESM"type": "module"
HTTP FrameworkFastify5.6.1
DI ContainerAwilix12.0.5 (listed, not used)
MessagingAzure Service Bus7.9.5
StorageAzure Blob Storage12.28.0
TestingVitest3.2.4
LoggingPino + CLA framework9.14.0

2.2 Service Architecture Type

This is a message-primary service with minimal HTTP exposure. The primary flow is:

Azure Service Bus (to-send topic)
↓
subscribe()
↓
attemptDelivery()
↓ (success) ↓ (failure)
complete() republishRetry()
↓
Azure Service Bus (to-retry topic)
↓
attemptRetryDelivery()

2.3 Project Structure

Project structure deviates from other Fluxx services and should be refactored to align application architecture. Particularly the prototype/ folder should be addressed (see 3.3 Medium Findings)

src/
├── index.ts # Application entry point
├── constants/ # Configuration keys, problem types, error codes
├── internal/ # Framework adapters, caching, app singleton
│ ├── frameworkAdapter/ # CLA DevCloud2 integration
│ └── pubsub/ # Message publisher factories
├── middleware/ # HTTP error handling (limited use)
├── prototype/ # Core message handling logic (22 files)
├── routes/ # HTTP routes (minimal)
├── services/ # Service Bus publisher factory
├── types/ # TypeScript type definitions
└── utils/ # Service Bus message utilities

2.4 Application Entry and Bootstrap

The entry point (src/index.ts) follows this sequence:

  1. Load dotenv configuration
  2. Create DevFramework instance
  3. Initialize Azure App Configuration
  4. Prepare configuration lookup function
  5. Register logger from framework
  6. Start Fastify server
  7. Start message subscribers (primary purpose)
  8. Register shutdown hooks

Observation: The service properly starts subscribers after the HTTP server, but the HTTP server serves almost no purpose (only a test endpoint exists)


3. Key Findings

3.1 Critical Findings

Awilix Declared But Not Used

Location: package.json lines 53, 56
Impact: Architectural debt; misleading dependency list
Description: Both @fastify/awilix (7.0.0) and awilix (12.0.5) are listed as production dependencies but are only imported in test files (test/unit/routes/postal.test.ts, test/contract/sql.test.ts). The production code does not use Awilix for dependency injection.

Evidence:

  • No createContainer() or asClass/asFunction calls in src/
  • The app object in src/internal/app.ts is a manual singleton pattern, not an Awilix container

In-Memory Retry Scheduling State

Location: src/prototype/attemptRetryDelivery.ts line 9
Impact: Data loss on pod restart; incorrect retry behavior in multi-replica deployments
Description: The scheduling store is created at module scope:

const schedulingStore = createSchedulingStore();

This creates an in-memory Record<string, RetryScheduling> that:

  • Is lost when the pod restarts
  • Is not shared across replicas
  • Could cause duplicate or missed deliveries in scaled environments

Global Module-Level Caches Without Bounds or Invalidation

Location:

  • src/internal/cached.ts (line 1: const cache = new Map())
  • src/internal/cachedForKey.ts (line 1: const cache = new Map<string, unknown>())

Impact: Memory leaks; stale data
Description: These caches:

  • Have no TTL or expiration
  • Have no maximum size
  • Cannot be invalidated
  • Will grow indefinitely in long-running processes

The cachedForKey cache stores deliverers keyed by clientApplicationId, which could grow unbounded if many clients are served.


3.2 High Findings

Swallowed Error Context in Message Handlers

Location: src/prototype/subscribeToSendTopic.ts lines 11-24

try {
await attemptDelivery(message);
} catch (_ignored) { // ← Error context discarded
const nextRetryScheduling = calculateNextRetryScheduling({...});
await republishRetry({...});
}

Impact: Loss of debugging information; inability to classify error types
Description: The error is caught but discarded as _ignored. There is no:

  • Logging of the original error
  • Classification (retryable vs. permanent)
  • Error context passed to retry scheduling

Hardcoded Topic Name in republishRetry

Location: src/prototype/republishRetry.ts line 12

await publish({ message: { ...message, retryScheduling: scheduling }, topicName: 'to-retry' });

Impact: Configuration inconsistency
Description: The topic name 'to-retry' is hardcoded rather than sourced from configuration constants, unlike the subscription names which use app.lookup().


Missing Message Validation at Subscription Boundary

Location: src/prototype/subscribe.ts lines 16-17

const postalMessage = message.body;
await messageHandler(postalMessage);

Impact: Runtime errors from malformed messages; no dead-letter handling for validation failures
Description: Messages are passed directly from Service Bus without validation. There is no:

  • Schema validation
  • Type guard verification
  • Dead-letter routing for invalid payloads

Service Missing Standard onClose Shutdown Hook

Location: src/index.ts
Impact: Implementation deviates from standard shutdown hooks.

Description: The service registers shutdown hooks via registerShutdownHook() for SIGINT/SIGTERM signals, but does not implement the Framework/Fluxx’s standard onClose hook

Expected Implementation:

frameworkApp.server.addHook('onClose', async () => {
const messageReceiver: MessageReceiver = frameworkApp.server.diContainer.resolve('messageReceiver') as MessageReceiver;
await messageReceiver.stopListening();
});

Recommendation: Implement the onClose hook to ensure proper cleanup during all shutdown scenarios, not just signal-based termination


3.3 Medium Findings

Subscription Handlers Should Be Lightly Refactored

Location: src/prototype/
Impact: Minor maintainability issues; small risk of hidden errors

Description: Both files follow a strong, consistent pattern, but:

  • Duplicate setup logic for app.lookup + subscribe exists in both files → Extract a small helper (e.g., createTopicSubscriber) to reduce repetition
  • Misleading comment in subscribeToRetryTopic.ts claims the handler is “intentionally empty,” but it isn’t → Remove or correct it
  • Swallowed errors in subscribeToSendTopic.ts (catch (_ignored)) hide real failures → At minimum, log the error
  • Redundant dateNow() calls in retry scheduling reduce clarity → Capture once and reuse

These are non-breaking refactors that will improve clarity and maintainability without changing behavior

Prototype Folder is Overloaded

Location: src/prototype/ (22 files)

Impact: Reduced maintainability; unclear architectural boundaries
Description: The prototype/ folder contains:

  • Core domain logic (retry scheduling)
  • Infrastructure (Service Bus client)
  • Application orchestration (subscribe, publish)
  • HTTP delivery (postMessage)
  • Time utilities (now.ts)

This mixes multiple architectural concerns in a single “prototype” folder name.


Inconsistent Configuration Key Usage

Location: Multiple files

Description: Configuration keys are inconsistently decleared/used:

  • ENV_KEYS in configuration.ts
    • NEW_INTERNAL_FILE_TOPIC_NAME defined, not used.
    • TOPIC_COMPLETED_RESULT defined, only used in contract test.
    • SUBSCRIPTION_FILE_PROCESSING_SERVICE defined, only used in contract test.
  • Environment variables defined in env.example and missing from configuration.ts
    • SUBSCRIPTION_TO_RETRY
    • SUBSCRIPTION_TO_SEND
    • TOPIC_TO_RETRY
    • TOPIC_TO_SEND
  • Environment variables accessed as string variables, rather than ENV_KEYS
    • 'TOPIC_TO_SEND' and 'SUBSCRIPTION_TO_SEND' used in subscribeToSendTopic.ts
    • 'TOPIC_TO_RETRY' and 'SUBSCRIPTION_TO_RETRY' used in subscribeToRetryTopic.ts
    • 'REGISTRATION_BASE_URL' used in fetchDeliveryUrl.ts

Fastify Routes Unused

Location: src/routes/v1/postal.ts

Impact: Architecture mismatch
Description: The only route is a test endpoint:

frameworkServer.get(`${basePath}/test/`, async (_request, reply) => {
reply.send('Test route is working!');
});

The service exposes no production HTTP endpoints, yet Fastify is fully bootstrapped.


Port Conflicts with Extractor Service

Location: Environment configuration (.env, .env.example)
Impact: Deployment conflicts; inability to run services simultaneously in local development

Description: The Postal service has port conflicts with the Extractor service:

  • APPLICATION_PORT (3400) conflicts with Extractor APPLICATION_PORT (3400)
  • DEBUG_PORT (9234) conflicts with Extractor DEBUG_PORT (9234)

Recommendation:

  • Assign unique ports to Postal service (e.g., APPLICATION_PORT=3500, DEBUG_PORT=9235)
  • Update all environment files (.env.example, .env.local.example)
  • Update documentation and Docker Compose configurations

Direct NodeJS Process Usage

Location: Multiple files
Impact: Testability issues; tight coupling to Node.js runtime

Description: Direct usage of the Node.js process object appears in multiple locations:

  • createAppConfigOptions.ts - Uses process.env directly
  • index.ts - Uses process.exitCode and process references
  • registerShutdownHook.ts - Uses process.on(), process.exitCode

Issues:

  • Makes unit testing more difficult (requires mocking global process)
  • Tight coupling to Node.js runtime environment
  • Inconsistent with framework’s configuration abstraction layer
  • Signal handling via process.on() bypasses Fastify’s lifecycle hooks

Recommendation:

  • Inject environment configuration through the CLA framework’s config lookup mechanism
  • Use Fastify lifecycle hooks (onClose, onReady) instead of raw process signals
  • If direct process access is necessary, wrap in testable abstractions
  • Consider using framework-provided exit code management

3.4 Low Findings

Comment Inaccuracy in attemptRetryDelivery

Location: src/prototype/subscribeToRetryTopic.ts lines 8-10

// This function is intentionally left empty.
// It is used to satisfy the type requirements of the subscribe function.
// The actual message handling is done in the attemptRetryDelivery function.

The function is not empty—it calls attemptRetryDelivery. This comment is misleading.


Duplicate Service Bus Client Management

Location:

  • src/prototype/provideServiceBusClient.ts
  • src/utils/serviceBusMessageUtils.ts

Description: Two different patterns for Service Bus clients:

  1. provideServiceBusClient() creates a cached singleton
  2. sendMessages() in utils creates a new client per call

This inconsistency could lead to connection pool exhaustion.


4. Dependency and Layer Analysis

4.1 Logical Layer Mapping

LayerExpected ContentsActual FoldersAssessment
PresentationRoutes, schemasroutes/, middleware/Mixed - minimal HTTP usage
ApplicationOrchestrationprototype/ (partial)Mixed - combined with domain
DomainBusiness logicprototype/ (partial), types/Mixed - no clear separation
InfrastructureAzure SDK adaptersprototype/, utils/, services/Poor - scattered
Cross-cuttingLogging, configinternal/, constants/Good

4.2 Dependency Direction Issues

IssueFromToViolation
Infrastructure in domainattemptRetryDelivery.tsrepublishRetry.ts → Azure SDKDomain knows about infrastructure
Mixed orchestrationsubscribeToSendTopic.tsBoth domain logic and infraApplication layer undefined

4.3 Coupling Assessment

ComponentCoupling LevelRisk
Azure Service Bus SDKRiskyUsed directly in multiple files without abstraction
AxiosAcceptableWrapped in postMessage.ts
CLA Framework packagesAcceptableAbstracted through internal/frameworkAdapter/
FastifyAcceptableMinimal coupling due to minimal HTTP usage

5. Testability Assessment

5.1 Test Organization

Test TypeLocationScope FilteringCount
Unittest/unit/TEST_SCOPE=unit~15 files
Contracttest/contract/TEST_SCOPE=contract3 files
Servicetest/service/TEST_SCOPE=service4 files

Strengths:

  • Clear separation of test scopes
  • Vitest configuration properly filters by TEST_SCOPE
  • Unit tests use mocking (vi.mock) appropriately

5.2 Testability Barriers

BarrierLocationImpact
Module-level cache instancescached.ts, cachedForKey.tsCaches persist across tests; requires module mocking
Module-level scheduling storeattemptRetryDelivery.tsState persists; requires mock factory
Global app singletoninternal/app.tsMust mock/stub for unit tests

5.3 Mocking Strategy

The codebase uses vi.mock() and vi.hoisted() patterns effectively:

// Example from attemptRetryDelivery.test.ts
const { mockSchedulingStore } = vi.hoisted(() => ({
mockSchedulingStore: {
getRetryScheduling: vi.fn(),
setRetryScheduling: vi.fn(),
},
}));
vi.mock('../../../src/prototype/createSchedulingStore.js', () => ({
createSchedulingStore: () => mockSchedulingStore,
}));

5.4 Test Coverage Recommendation

% Coverage report from v8

File% Stmts% Branch% Funcs% Lines
All files78.8192.2567.7478.81

Overall coverage shows strong branch coverage at 92.25%, but lower statement and line coverage at 78.81% and function coverage at 67.74%. This indicates that while most conditional paths are evaluated, many executable lines and functions are not fully exercised.

The primary gaps are in untested or partially tested functions, likely including error handling, edge cases, fallback logic, and less frequently used utilities. The difference between branch and line coverage suggests that some code within covered branches is not being executed.

Priority should be given to:

  • Uncovered or partially covered functions.
  • Error and exception paths.
  • Boundary and edge cases.
  • Optional or configuration-driven logic.

Improving these areas will increase confidence in code reliability and reduce the risk of defects in untested paths.

While overall coverage is decent (78.81% statements/lines, 92.25% branches), function coverage remains low at 67.74%, and several critical areas have significant gaps. Notably, src/internal and src/prototype contain files with 0–50% coverage (e.g., createAppConfigOptions.ts, cachedForKey.ts, postMessage.ts, registerShutdownHook.ts, subscribe.ts). These modules likely represent important runtime behavior and integration logic.

We should prioritize adding unit tests for these low-coverage files and uncovered line ranges to improve function coverage, reduce risk in core logic, and strengthen confidence in refactors and future feature work.