Skip to content

Architecture: Encapsulate Queue admission in QueueManager.enqueue to eliminate canEnqueue seam leak #136

Description

@jonbaldie

Candidate Summary

  • Recommendation Strength: Strong
  • Modules Involved: src/manager.ts, src/handler.ts
  • Domain Context: Queue Manager, Queue, Enqueue, Item Lifecycle
  • Architecture Vocabulary: module, interface, depth, seam, leverage, locality

Problem & Evidence

Currently, Queue Manager exposes a query method canEnqueue(name: string): boolean alongside the mutating method enqueue(name: string, payload: T): Manager<T>.

In src/handler.ts:

if (!mgr.canEnqueue(queueName)) {
    return new Response("Queue full or too many queues", { status: 507 });
}
mgr.enqueue(queueName, json.payload);

This creates an awkward two-step handshake across the seam for every caller attempting to enqueue a payload:

  1. Duplicated validation & limit checks: canEnqueue validates the queue name, checks canCreateQueue(), and inspects queueDepthLimit. Then enqueue repeats validateName, find(name), canCreateQueue(), and queue.length >= this.queueDepthLimit.
  2. Leaky error contract: enqueue throws generic untyped Error("Queue count limit reached") and Error("Queue depth limit reached"). In src/handler.ts, enqueueHandler only catches SyntaxError, UnsupportedNumberError, and QueueNameTooLongError in enqueueErrorResponse. If mgr.enqueue throws a capacity limit error (e.g. under concurrent operations), the error is unhandled by the error response mapper and becomes an uncaught 500 Internal Server Error instead of a 507.
  3. Callers are forced to know internal ordering invariants (they must call canEnqueue first) because enqueue does not provide a typed domain error contract for capacity admission failure.

Test Evidence (/deintrovert-tests)

In tests/manager_test.ts, unit tests test canEnqueue in isolation as a separate query seam, verifying boolean flags rather than observing the SUT's atomic state transitions and mutations. In tests/handler_test.ts, the handler is tested with full HTTP requests to verify 507 responses. Deleting canEnqueue aligns the test surface with the public interface: tests exercise enqueue directly and assert on the resulting queue state and thrown domain errors.

The Deletion Test

Applying the deletion test to canEnqueue: deleting canEnqueue concentrates complexity rather than scattering it. Queue Manager becomes a deeper module: callers interact solely with enqueue(), which owns admission control atomically.

Proposed Change

  1. Define a domain error in src/manager.ts:
export class QueueCapacityExceededError extends Error {
    constructor(message = "Queue full or too many queues") {
        super(message);
        this.name = "QueueCapacityExceededError";
    }
}
  1. Have QueueManager.enqueue(name, payload) throw QueueCapacityExceededError whenever queueDepthLimit or queueCountLimit would be exceeded.
  2. Remove canEnqueue from QueueManager's public interface.
  3. In src/handler.ts, call mgr.enqueue(queueName, json.payload) directly in enqueueHandler, and catch QueueCapacityExceededError in enqueueErrorResponse to return HTTP 507.

Benefits

  • Leverage: Callers learn and invoke a single method enqueue instead of coordinating query-then-mutate handshakes.
  • Locality: Capacity validation, name validation, and queue lifecycle management concentrate inside QueueManager.
  • Testability: The interface is the test surface. Tests verify capacity limits by exercising enqueue and observing genuine state transitions.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestready-for-agentFully specified and ready for an implementation agent

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions