From 7d9c102c6de35bef713b70ed326c061976a66a8e Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 3 Feb 2026 20:21:06 -0600 Subject: [PATCH] fix(#275): Prevent silent connection initiation failures Fixed silent connection initiation failures where HTTP errors were caught but success was returned to the user, leaving zombie connections in PENDING state forever. Changes: - Delete failed connection from database when HTTP request fails - Throw BadRequestException with clear error message - Added test to verify connection deletion and exception throwing - Import BadRequestException in connection.service.ts User Impact: - Users now receive immediate feedback when connection initiation fails - No more zombie connections stuck in PENDING state - Clear error messages indicate the reason for failure Testing: - Added test case: "should delete connection and throw error if request fails" - All 21 connection service tests passing - Quality gates: lint, typecheck, build all passing Fixes #275 Co-Authored-By: Claude Sonnet 4.5 --- .../src/federation/connection.service.spec.ts | 31 +++++++ apps/api/src/federation/connection.service.ts | 14 +++- .../275-silent-connection-failures.md | 82 +++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 docs/scratchpads/275-silent-connection-failures.md diff --git a/apps/api/src/federation/connection.service.spec.ts b/apps/api/src/federation/connection.service.spec.ts index 1fd4930..cd8e4fd 100644 --- a/apps/api/src/federation/connection.service.spec.ts +++ b/apps/api/src/federation/connection.service.spec.ts @@ -85,6 +85,7 @@ describe("ConnectionService", () => { findUnique: vi.fn(), findMany: vi.fn(), update: vi.fn(), + delete: vi.fn(), }, }, }, @@ -208,6 +209,36 @@ describe("ConnectionService", () => { }) ); }); + + it("should delete connection and throw error if request fails", async () => { + const mockAxiosResponse: AxiosResponse = { + data: mockRemoteIdentity, + status: 200, + statusText: "OK", + headers: {}, + config: {} as never, + }; + + vi.spyOn(httpService, "get").mockReturnValue(of(mockAxiosResponse)); + vi.spyOn(httpService, "post").mockReturnValue( + throwError(() => new Error("Connection refused")) + ); + const createSpy = vi + .spyOn(prismaService.federationConnection, "create") + .mockResolvedValue(mockConnection); + const deleteSpy = vi + .spyOn(prismaService.federationConnection, "delete") + .mockResolvedValue(mockConnection); + + await expect(service.initiateConnection(mockWorkspaceId, mockRemoteUrl)).rejects.toThrow( + "Failed to initiate connection" + ); + + expect(createSpy).toHaveBeenCalled(); + expect(deleteSpy).toHaveBeenCalledWith({ + where: { id: mockConnection.id }, + }); + }); }); describe("acceptConnection", () => { diff --git a/apps/api/src/federation/connection.service.ts b/apps/api/src/federation/connection.service.ts index b2dae79..93b7630 100644 --- a/apps/api/src/federation/connection.service.ts +++ b/apps/api/src/federation/connection.service.ts @@ -10,6 +10,7 @@ import { NotFoundException, UnauthorizedException, ServiceUnavailableException, + BadRequestException, } from "@nestjs/common"; import { HttpService } from "@nestjs/axios"; import { FederationConnectionStatus, Prisma } from "@prisma/client"; @@ -68,7 +69,7 @@ export class ConnectionService { const signature = await this.signatureService.signMessage(request); const signedRequest: ConnectionRequest = { ...request, signature }; - // Send connection request to remote instance (fire-and-forget for now) + // Send connection request to remote instance try { await firstValueFrom( this.httpService.post(`${remoteUrl}/api/v1/federation/incoming/connect`, signedRequest) @@ -76,7 +77,16 @@ export class ConnectionService { this.logger.log(`Connection request sent to ${remoteUrl}`); } catch (error) { this.logger.error(`Failed to send connection request to ${remoteUrl}`, error); - // Connection is still created in PENDING state, can be retried + + // Delete the failed connection to prevent zombie connections in PENDING state + await this.prisma.federationConnection.delete({ + where: { id: connection.id }, + }); + + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + throw new BadRequestException( + `Failed to initiate connection to ${remoteUrl}: ${errorMessage}` + ); } return this.mapToConnectionDetails(connection); diff --git a/docs/scratchpads/275-silent-connection-failures.md b/docs/scratchpads/275-silent-connection-failures.md new file mode 100644 index 0000000..eeddb6b --- /dev/null +++ b/docs/scratchpads/275-silent-connection-failures.md @@ -0,0 +1,82 @@ +# Issue #275: Fix silent connection initiation failures + +## Objective + +Fix silent connection initiation failures where HTTP errors are caught but success is returned to the user, leaving zombie connections in PENDING state forever. + +## Location + +`apps/api/src/federation/connection.service.ts:72-80` + +## Problem + +Current code: + +```typescript +try { + await firstValueFrom( + this.httpService.post(`${remoteUrl}/api/v1/federation/incoming/connect`, signedRequest) + ); + this.logger.log(`Connection request sent to ${remoteUrl}`); +} catch (error) { + this.logger.error(`Failed to send connection request to ${remoteUrl}`, error); + // Connection is still created in PENDING state, can be retried +} + +return this.mapToConnectionDetails(connection); +``` + +Issues: + +- Catches HTTP failures but returns success +- Connection stays in PENDING state forever +- Creates zombie connections +- User sees success message but connection actually failed + +## Solution + +1. Delete the failed connection from database +2. Throw exception with clear error message +3. User gets immediate feedback that connection failed + +## Implementation + +```typescript +try { + await firstValueFrom( + this.httpService.post(`${remoteUrl}/api/v1/federation/incoming/connect`, signedRequest) + ); + this.logger.log(`Connection request sent to ${remoteUrl}`); +} catch (error) { + this.logger.error(`Failed to send connection request to ${remoteUrl}`, error); + + // Delete the failed connection to prevent zombie connections + await this.prisma.federationConnection.delete({ + where: { id: connection.id }, + }); + + throw new BadRequestException( + `Failed to initiate connection to ${remoteUrl}: ${error instanceof Error ? error.message : "Unknown error"}` + ); +} +``` + +## Testing + +Test scenarios: + +1. Remote instance is unreachable - should throw exception and delete connection +2. Remote instance returns error - should throw exception and delete connection +3. Remote instance times out - should throw exception and delete connection +4. Remote instance returns success - should create connection in PENDING state + +## Progress + +- [ ] Create scratchpad +- [ ] Implement fix in connection.service.ts +- [ ] Add/update tests +- [ ] Run quality gates +- [ ] Commit changes +- [ ] Create PR +- [ ] Merge to develop +- [ ] Close issue #275