fix: Bug - Images and files in chat doesn't load

This commit is contained in:
2026-07-14 11:27:19 +02:00
parent 41ebaf2407
commit 20d7f22fd2
21 changed files with 631 additions and 38 deletions
@@ -0,0 +1,48 @@
import '@angular/compiler';
import { vi } from 'vitest';
import { buildAttachmentDisplayPinKey } from '../../domain/logic/attachment-blob-eviction.rules';
import type { Attachment } from '../../domain/models/attachment.model';
import { AttachmentManagerService } from './attachment-manager.service';
describe('AttachmentManagerService display hydration lifecycle', () => {
it('cancels and revokes unpinned attachments while preserving pinned fullscreen media', () => {
const unpinned: Attachment = {
id: 'att-1',
messageId: 'msg-1',
filename: 'photo.png',
size: 3,
mime: 'image/png',
isImage: true,
available: true,
savedPath: '/appdata/photo.png',
objectUrl: 'blob:http://localhost/photo'
};
const pinned: Attachment = {
...unpinned,
id: 'att-2',
objectUrl: 'blob:http://localhost/pinned'
};
const persistence = {
cancelDisplayHydration: vi.fn(),
revokeAttachmentDisplayBlob: vi.fn(() => true)
};
const runtimeStore = {
getAttachmentsForMessage: vi.fn(() => [unpinned, pinned]),
touch: vi.fn()
};
const manager = Object.create(AttachmentManagerService.prototype) as AttachmentManagerService;
Reflect.set(manager, 'persistence', persistence);
Reflect.set(manager, 'runtimeStore', runtimeStore);
Reflect.set(manager, 'pinnedDisplayBlobKeys', new Set([buildAttachmentDisplayPinKey('msg-1', 'att-2')]));
manager.revokeOffscreenDisplayBlobsForMessage('msg-1');
expect(persistence.cancelDisplayHydration).toHaveBeenCalledTimes(1);
expect(persistence.cancelDisplayHydration).toHaveBeenCalledWith(unpinned);
expect(persistence.revokeAttachmentDisplayBlob).toHaveBeenCalledTimes(1);
expect(persistence.revokeAttachmentDisplayBlob).toHaveBeenCalledWith(unpinned);
expect(runtimeStore.touch).toHaveBeenCalledTimes(1);
});
});
@@ -13,6 +13,7 @@ import { yieldToAttachmentHydrationLoop } from '../../domain/logic/attachment-bl
import {
buildAttachmentDisplayPinKey,
collectMessageIdsForInactiveRoomBlobRelease,
isAttachmentDisplayPinned,
shouldRevokeDisplayBlobForAttachment
} from '../../domain/logic/attachment-blob-eviction.rules';
import {
@@ -214,6 +215,8 @@ export class AttachmentManagerService {
return;
}
this.cancelDisplayHydrationForMessage(messageId);
let hasChanges = false;
for (const attachment of this.runtimeStore.getAttachmentsForMessage(messageId)) {
@@ -231,6 +234,20 @@ export class AttachmentManagerService {
}
}
cancelDisplayHydrationForMessage(messageId: string): void {
if (!messageId) {
return;
}
for (const attachment of this.runtimeStore.getAttachmentsForMessage(messageId)) {
if (isAttachmentDisplayPinned(messageId, attachment.id, this.pinnedDisplayBlobKeys)) {
continue;
}
this.persistence.cancelDisplayHydration(attachment);
}
}
releaseDisplayBlobsForInactiveRooms(activeRoomId: string | null): void {
const messageIds = collectMessageIdsForInactiveRoomBlobRelease(
Array.from(this.runtimeStore.getAttachmentEntries(), ([messageId]) => messageId),
@@ -256,9 +273,11 @@ export class AttachmentManagerService {
}
handleFileAnnounce(payload: FileAnnouncePayload): void {
const isNew = this.transfer.handleFileAnnounce(payload);
this.transfer.handleFileAnnounce(payload);
if (isNew && payload.messageId && payload.file?.id) {
if (payload.messageId && payload.file?.id) {
// Re-announces are recovery signals too: a host may have come back after
// an earlier file-not-found, so re-run the guarded auto-download path.
this.queueAutoDownloadsForMessage(payload.messageId, payload.file.id);
}
}
@@ -98,6 +98,58 @@ describe('AttachmentPersistenceService', () => {
expect(attachmentStorage.getFileSize).not.toHaveBeenCalled();
});
it('preserves a completed runtime download when database hydration finishes later', async () => {
const injector = Injector.create({
providers: [
AttachmentPersistenceService,
AttachmentRuntimeStore,
{ provide: DatabaseService, useValue: database },
{ provide: AttachmentStorageService, useValue: attachmentStorage },
{ provide: Store, useValue: { select: () => of('room-1') } }
]
});
const service = runInInjectionContext(injector, () => injector.get(AttachmentPersistenceService));
const runtimeStore = injector.get(AttachmentRuntimeStore);
const completedDownload = {
id: 'att-1',
messageId: 'msg-1',
filename: 'photo.png',
size: 1_500_000,
mime: 'image/png',
isImage: true,
available: true,
objectUrl: 'blob:http://localhost/completed',
receivedBytes: 1_500_000
};
const announcedDuringStartup = {
id: 'att-live',
messageId: 'msg-live',
filename: 'new-photo.png',
size: 512,
mime: 'image/png',
isImage: true,
available: false,
receivedBytes: 0
};
runtimeStore.setAttachmentsForMessage('msg-1', [completedDownload]);
runtimeStore.setAttachmentsForMessage('msg-live', [announcedDuringStartup]);
await service.initFromDatabase();
const restored = runtimeStore.getAttachmentsForMessage('msg-1')[0];
expect(restored).toBe(completedDownload);
expect(restored).toMatchObject({
available: true,
objectUrl: 'blob:http://localhost/completed',
receivedBytes: 1_500_000,
savedPath: '/appdata/photo.png'
});
expect(runtimeStore.getAttachmentsForMessage('msg-live')[0]).toBe(announcedDuringStartup);
});
it('hydrates blob URLs on demand for a single attachment', async () => {
const injector = Injector.create({
providers: [
@@ -134,6 +186,144 @@ describe('AttachmentPersistenceService', () => {
expect(attachmentStorage.readFile).not.toHaveBeenCalled();
});
it('deduplicates concurrent display hydration for the same attachment', async () => {
attachmentStorage.canReadFileChunks.mockReturnValue(false);
const service = createService();
const attachment = {
id: 'att-1',
messageId: 'msg-1',
filename: 'photo.png',
size: 3,
mime: 'image/png',
isImage: true,
savedPath: '/appdata/photo.png',
available: false
};
const [first, second] = await Promise.all([service.ensureInlineDisplayObjectUrl(attachment), service.ensureInlineDisplayObjectUrl(attachment)]);
expect(first).toBe(true);
expect(second).toBe(true);
expect(attachmentStorage.readFile).toHaveBeenCalledTimes(1);
});
it('limits concurrent display hydration to two attachments', async () => {
attachmentStorage.canReadFileChunks.mockReturnValue(false);
const pendingReads: ((base64: string) => void)[] = [];
attachmentStorage.readFile.mockImplementation(() => new Promise<string>((resolve) => {
pendingReads.push(resolve);
}));
const service = createService();
const attachments = Array.from({ length: 3 }, (_, index) => ({
id: `att-${index + 1}`,
messageId: `msg-${index + 1}`,
filename: `photo-${index + 1}.png`,
size: 3,
mime: 'image/png',
isImage: true,
savedPath: `/appdata/photo-${index + 1}.png`,
available: false
}));
const hydrations = attachments.map((attachment) => service.ensureInlineDisplayObjectUrl(attachment));
await vi.waitFor(() => expect(attachmentStorage.readFile).toHaveBeenCalledTimes(2));
expect(pendingReads).toHaveLength(2);
pendingReads.shift()?.('QUJD');
await vi.waitFor(() => expect(attachmentStorage.readFile).toHaveBeenCalledTimes(3));
for (const resolve of pendingReads) {
resolve('QUJD');
}
await expect(Promise.all(hydrations)).resolves.toEqual([
true,
true,
true
]);
});
it('cancels an in-flight hydration before it can attach a stale blob URL', async () => {
let finishChunkRead!: (base64: string) => void;
attachmentStorage.getFileSize.mockResolvedValue(3);
attachmentStorage.readFileChunk.mockImplementation(() => new Promise<string>((resolve) => {
finishChunkRead = resolve;
}));
const service = createService();
const attachment = {
id: 'att-1',
messageId: 'msg-1',
filename: 'photo.png',
size: 3,
mime: 'image/png',
isImage: true,
savedPath: '/appdata/photo.png',
available: false
};
const createObjectUrlSpy = vi.spyOn(URL, 'createObjectURL');
const hydration = service.ensureInlineDisplayObjectUrl(attachment);
await vi.waitFor(() => expect(attachmentStorage.readFileChunk).toHaveBeenCalledTimes(1));
service.cancelDisplayHydration(attachment);
finishChunkRead('QUJD');
await expect(hydration).resolves.toBe(false);
expect(attachment.objectUrl).toBeUndefined();
expect(createObjectUrlSpy).not.toHaveBeenCalled();
createObjectUrlSpy.mockRestore();
});
it('allows a newer hydration to supersede cancelled stale work', async () => {
attachmentStorage.canReadFileChunks.mockReturnValue(false);
let finishStaleRead!: (base64: string) => void;
let readCount = 0;
attachmentStorage.readFile.mockImplementation(() => {
readCount++;
if (readCount === 1) {
return new Promise<string>((resolve) => {
finishStaleRead = resolve;
});
}
return Promise.resolve('REVG');
});
const service = createService();
const attachment = {
id: 'att-1',
messageId: 'msg-1',
filename: 'photo.png',
size: 3,
mime: 'image/png',
isImage: true,
savedPath: '/appdata/photo.png',
available: false
};
const staleHydration = service.ensureInlineDisplayObjectUrl(attachment);
await vi.waitFor(() => expect(attachmentStorage.readFile).toHaveBeenCalledTimes(1));
service.cancelDisplayHydration(attachment);
const currentHydration = service.ensureInlineDisplayObjectUrl(attachment);
await expect(currentHydration).resolves.toBe(true);
finishStaleRead('QUJD');
await expect(staleHydration).resolves.toBe(false);
expect(attachmentStorage.readFile).toHaveBeenCalledTimes(2);
expect(attachment.objectUrl).toMatch(/^blob:/);
});
it('does not duplicate disk-hydrated bytes into the original-file cache', async () => {
const injector = Injector.create({
providers: [
@@ -281,13 +471,24 @@ describe('AttachmentPersistenceService', () => {
isImage: true,
savedPath: '/appdata/photo.png',
available: true,
objectUrl: 'blob:http://localhost/abc'
objectUrl: 'blob:http://localhost/abc',
receivedBytes: 3,
speedBps: 512,
startedAtMs: 100,
lastUpdateMs: 200
};
const revokeSpy = vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => undefined);
expect(service.revokeAttachmentDisplayBlob(attachment)).toBe(true);
expect(attachment.objectUrl).toBeUndefined();
expect(attachment.savedPath).toBe('/appdata/photo.png');
expect(attachment).toMatchObject({
receivedBytes: 0,
speedBps: 0,
startedAtMs: undefined,
lastUpdateMs: undefined
});
expect(revokeSpy).toHaveBeenCalledWith('blob:http://localhost/abc');
revokeSpy.mockRestore();
@@ -17,9 +17,19 @@ import { mergeAttachmentLocalPaths } from '../../domain/logic/attachment-persist
import { isAttachmentMedia } from '../../domain/logic/attachment.logic';
import { AttachmentRuntimeStore } from './attachment-runtime.store';
const MAX_CONCURRENT_DISPLAY_HYDRATIONS = 2;
interface DisplayHydrationTask {
cancelled: boolean;
promise: Promise<boolean>;
}
@Injectable({ providedIn: 'root' })
export class AttachmentPersistenceService {
private initPromise: Promise<void> | null = null;
private activeDisplayHydrations = 0;
private readonly displayHydrationQueue: (() => void)[] = [];
private readonly displayHydrations = new Map<string, DisplayHydrationTask>();
private readonly runtimeStore = inject(AttachmentRuntimeStore);
private readonly ngrxStore = inject(Store);
@@ -33,6 +43,8 @@ export class AttachmentPersistenceService {
const savedPathsToDelete = new Set<string>();
for (const attachment of attachments) {
this.cancelDisplayHydration(attachment);
if (attachment.objectUrl) {
try {
URL.revokeObjectURL(attachment.objectUrl);
@@ -133,13 +145,34 @@ export class AttachmentPersistenceService {
return this.ensurePersistedUploadHost(attachment, { hydrateMediaForDisplay: false });
}
cancelDisplayHydration(attachment: Pick<Attachment, 'id' | 'messageId'>): void {
const hydrationKey = this.buildDisplayHydrationKey(attachment);
const task = this.displayHydrations.get(hydrationKey);
if (!task) {
return;
}
task.cancelled = true;
if (this.displayHydrations.get(hydrationKey) === task) {
this.displayHydrations.delete(hydrationKey);
}
}
revokeAttachmentDisplayBlob(attachment: Attachment): boolean {
this.cancelDisplayHydration(attachment);
if (!canRevokeAttachmentDisplayBlob(attachment)) {
return false;
}
this.revokeAttachmentObjectUrl(attachment);
attachment.objectUrl = undefined;
attachment.receivedBytes = 0;
attachment.speedBps = 0;
attachment.startedAtMs = undefined;
attachment.lastUpdateMs = undefined;
// Once the bytes live on disk, the cached File duplicate is redundant:
// peer requests are served from the disk path and re-display rehydrates
@@ -199,19 +232,64 @@ export class AttachmentPersistenceService {
return true;
}
async ensureInlineDisplayObjectUrl(attachment: Attachment): Promise<boolean> {
ensureInlineDisplayObjectUrl(attachment: Attachment): Promise<boolean> {
if (!needsBlobObjectUrlForInlineDisplay(attachment.objectUrl)) {
return true;
return Promise.resolve(true);
}
const hydrationKey = this.buildDisplayHydrationKey(attachment);
const existingTask = this.displayHydrations.get(hydrationKey);
if (existingTask) {
return existingTask.promise;
}
const task: DisplayHydrationTask = {
cancelled: false,
promise: Promise.resolve(false)
};
this.displayHydrations.set(hydrationKey, task);
const scheduled = this.scheduleDisplayHydration(
task,
() => this.runInlineDisplayHydration(attachment, hydrationKey, task)
);
task.promise = scheduled.finally(() => {
if (this.displayHydrations.get(hydrationKey) === task) {
this.displayHydrations.delete(hydrationKey);
}
});
return task.promise;
}
private async runInlineDisplayHydration(
attachment: Attachment,
hydrationKey: string,
task: DisplayHydrationTask
): Promise<boolean> {
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
let diskPath = await this.attachmentStorage.resolveExistingPath(attachment);
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
if (!diskPath) {
const roomName = await this.resolveStorageContainerName(attachment);
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
diskPath = await this.attachmentStorage.resolveCanonicalStoredPath(attachment, roomName);
if (diskPath) {
if (diskPath && this.isDisplayHydrationCurrent(hydrationKey, task)) {
attachment.savedPath = diskPath;
void this.persistAttachmentMeta(attachment);
}
@@ -224,7 +302,7 @@ export class AttachmentPersistenceService {
if (this.attachmentStorage.providesInlineObjectUrl()) {
const nativeUrl = await this.attachmentStorage.getFileUrl(diskPath);
if (nativeUrl) {
if (nativeUrl && this.isDisplayHydrationCurrent(hydrationKey, task)) {
this.revokeAttachmentObjectUrl(attachment);
attachment.objectUrl = nativeUrl;
attachment.available = true;
@@ -233,9 +311,18 @@ export class AttachmentPersistenceService {
}
}
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
this.revokeAttachmentObjectUrl(attachment);
const restored = await this.restoreAttachmentBlobFromDiskPath(attachment, diskPath);
const restored = await this.restoreAttachmentBlobFromDiskPath(
attachment,
diskPath,
hydrationKey,
task
);
return restored;
}
@@ -302,14 +389,29 @@ export class AttachmentPersistenceService {
private async loadFromDatabase(): Promise<void> {
try {
const allRecords: AttachmentMeta[] = await this.database.getAllAttachments();
const grouped = new Map<string, Attachment[]>();
const grouped = new Map<string, Attachment[]>(
Array.from(
this.runtimeStore.getAttachmentEntries(),
([messageId, attachments]) => [messageId, [...attachments]]
)
);
for (const record of allRecords) {
const attachment: Attachment = { ...record,
available: false };
const bucket = grouped.get(record.messageId) ?? [];
const runtimeAttachment = bucket.find((attachment) => attachment.id === record.id);
if (runtimeAttachment) {
const localPaths = mergeAttachmentLocalPaths(runtimeAttachment, record);
runtimeAttachment.filePath = localPaths.filePath ?? undefined;
runtimeAttachment.savedPath = localPaths.savedPath ?? undefined;
} else {
const attachment: Attachment = { ...record,
available: false };
bucket.push(attachment);
}
bucket.push(attachment);
grouped.set(record.messageId, bucket);
}
@@ -350,11 +452,16 @@ export class AttachmentPersistenceService {
await this.migrateFromLocalStorage();
}
private async restoreAttachmentBlobFromDiskPath(attachment: Attachment, diskPath: string): Promise<boolean> {
private async restoreAttachmentBlobFromDiskPath(
attachment: Attachment,
diskPath: string,
hydrationKey: string,
task: DisplayHydrationTask
): Promise<boolean> {
if (this.attachmentStorage.canReadFileChunks()) {
const fileSize = await this.attachmentStorage.getFileSize(diskPath);
if (!fileSize || fileSize < 1) {
if (!fileSize || fileSize < 1 || !this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
@@ -364,7 +471,7 @@ export class AttachmentPersistenceService {
const end = Math.min(start + ATTACHMENT_BLOB_READ_CHUNK_SIZE_BYTES, fileSize);
const chunkBase64 = await this.attachmentStorage.readFileChunk(diskPath, start, end);
if (!chunkBase64) {
if (!chunkBase64 || !this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
@@ -375,18 +482,26 @@ export class AttachmentPersistenceService {
}
}
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
this.applyAttachmentBlob(attachment, new Blob(blobParts as BlobPart[], { type: attachment.mime }));
return true;
}
const base64 = await this.attachmentStorage.readFile(diskPath);
if (!base64) {
if (!base64 || !this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
const bytes = decodeBase64ToUint8Array(base64);
if (!this.isDisplayHydrationCurrent(hydrationKey, task)) {
return false;
}
this.applyAttachmentBlob(
attachment,
new Blob([bytes.buffer as ArrayBuffer], { type: attachment.mime })
@@ -395,6 +510,52 @@ export class AttachmentPersistenceService {
return true;
}
private scheduleDisplayHydration(
task: DisplayHydrationTask,
hydrate: () => Promise<boolean>
): Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
this.displayHydrationQueue.push(() => {
if (task.cancelled) {
resolve(false);
this.drainDisplayHydrationQueue();
return;
}
this.activeDisplayHydrations++;
void hydrate()
.then(resolve, reject)
.finally(() => {
this.activeDisplayHydrations--;
this.drainDisplayHydrationQueue();
});
});
this.drainDisplayHydrationQueue();
});
}
private drainDisplayHydrationQueue(): void {
while (
this.activeDisplayHydrations < MAX_CONCURRENT_DISPLAY_HYDRATIONS &&
this.displayHydrationQueue.length > 0
) {
this.displayHydrationQueue.shift()?.();
}
}
private isDisplayHydrationCurrent(
hydrationKey: string,
task: DisplayHydrationTask
): boolean {
return !task.cancelled && this.displayHydrations.get(hydrationKey) === task;
}
private buildDisplayHydrationKey(attachment: Pick<Attachment, 'id' | 'messageId'>): string {
return `${attachment.messageId}:${attachment.id}`;
}
// The blob always comes from a disk path here, so peers are served from
// disk and no original-file copy is cached; caching one would keep a second
// full copy of the bytes alive for the whole session.
@@ -986,6 +986,43 @@ describe('AttachmentTransferService', () => {
expect(service.hasPendingRequest(MESSAGE_ID, FILE_ID)).toBe(false);
});
it('notifies attachment views when an outbound request starts', async () => {
const service = createService();
const attachment = registerIncomingAttachment(3_000);
const versionBeforeRequest = runtimeStore.updated();
await service.requestFromAnyPeer(MESSAGE_ID, attachment);
expect(runtimeStore.updated()).toBeGreaterThan(versionBeforeRequest);
expect(service.hasPendingRequest(MESSAGE_ID, FILE_ID)).toBe(true);
expect(webrtc.sendToPeer).toHaveBeenCalledWith(PEER_ID, expect.objectContaining({
type: 'file-request'
}));
});
it('surfaces a retry error when an async request race exhausts every peer', async () => {
let finishLocalRestore!: (restored: boolean) => void;
persistence.tryRestoreAttachmentFromLocal.mockImplementation(() => new Promise<boolean>((resolve) => {
finishLocalRestore = resolve;
}));
const service = createService();
const attachment = registerIncomingAttachment(3_000);
const request = service.requestFromAnyPeer(MESSAGE_ID, attachment);
service.handleFileNotFound({
messageId: MESSAGE_ID,
fileId: FILE_ID
});
finishLocalRestore(false);
await request;
expect(service.hasPendingRequest(MESSAGE_ID, FILE_ID)).toBe(false);
expect(attachment.requestError).toBe('attachment.errors.fileNotFound');
});
it('normalizes generic octet-stream announces into image metadata for gallery grouping', () => {
const service = createService();
@@ -169,12 +169,14 @@ export class AttachmentTransferService {
async requestFromAnyPeer(messageId: string, attachment: Attachment): Promise<void> {
const requestKey = this.buildRequestKey(messageId, attachment.id);
const clearedRequestError = this.clearAttachmentRequestError(attachment);
this.clearAttachmentRequestError(attachment);
// Mark the request pending synchronously so concurrent triggers (file-announce,
// message sync, peer connect) cannot double-request the same file - a duplicate
// request makes the sender stream the file twice and corrupts byte accounting.
this.runtimeStore.setPendingRequestPeers(requestKey, new Set<string>());
this.runtimeStore.touch();
if (needsAttachmentDisplayHydration(attachment)) {
const hydratedLocally = await this.persistence.tryRestoreAttachmentFromLocal(attachment);
@@ -222,10 +224,12 @@ export class AttachmentTransferService {
return;
}
if (clearedRequestError)
this.runtimeStore.touch();
const didSendRequest = this.sendFileRequestToNextPeer(messageId, attachment.id, attachment.uploaderPeerId);
this.sendFileRequestToNextPeer(messageId, attachment.id, attachment.uploaderPeerId);
if (!didSendRequest) {
attachment.requestError = this.appI18n.instant(FILE_NOT_FOUND_REQUEST_ERROR_KEY);
this.runtimeStore.touch();
}
}
handleFileNotFound(payload: FileNotFoundPayload): void {