utopia-php / utopia-php/monorepo
Consumer::fetch()'s unsubscribe in finally can replace the exception it was added for
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 3
- Forks
- 4
- Avg merge
- 12h 25m
- Merged PRs (30d)
- 103
Description
Consumer::fetch() moved its unsubscribe into a finally so the inbox subscription is not leaked on a throw (a real fix — attemptReconnect() re-subscribes every leaked sid, so the leak compounded across a reconnect storm):
// packages/nats/src/JetStream/Consumer.php:59
try {
$this->conn->publish($requestSubject, $payload, $inbox);
$this->collectBatch($sub, $messageBatch, $batch, $timeout);
} finally {
$sub->unsubscribe();
}
Subscription::unsubscribe() reaches Connection::unsubscribe() → Connection::send(), which on a dead socket buffers, attempts a reconnect, and raises ConnectionException('Failed to reconnect to any NATS server') once attempts are exhausted.
In PHP an exception thrown from a finally replaces the one already in flight. So on the exact path the finally was added for — the connection died mid-fetch — the useful error (the -ERR, the JetStreamException, or the original ConnectionException naming what actually happened) is swapped for a generic reconnect failure. The subscription is still correctly cleaned up; only the diagnosis is lost, at the moment it matters most.
Fix: suppress a throw from unsubscribe() when an exception is already propagating — the connection is being torn down anyway, so the unsubscribe has nothing left to accomplish — or capture the original and rethrow it.
Found by a review sweep over main while resolving #192; not introduced by it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/nats/src/JetStream/Consumer.php at Consumer::fetch() and trace Subscription::unsubscribe() through Connection::unsubscribe() to Connection::send(). Reproduce or cover a fetch failure during teardown, then verify the original exception remains visible while cleanup still occurs without replacing it with a reconnect failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100