From e78911e8505d6687faf75061763b304d611b4917 Mon Sep 17 00:00:00 2001 From: yusufipk Date: Tue, 18 Aug 2026 11:07:25 +0300 Subject: [PATCH] fix(comments): say the account is full instead of blaming the comment The attachment goes up before the comment does, so a full account fails on the image and never reaches the comment at all. Both that failure and a rejected comment came out as "Failed to add comment", which tells the uploader to try again, and trying again is the one thing that cannot work when there is no room left. Both now read out what the server said. A network fault, which has no message anybody wants to see, still falls back to the old line. --- .../video-page/hooks/use-comment-actions.ts | 25 +++++-- .../hooks/use-comment-actions.test.ts | 68 ++++++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/components/video-page/hooks/use-comment-actions.ts b/components/video-page/hooks/use-comment-actions.ts index 02cc6cd..02ce4e4 100644 --- a/components/video-page/hooks/use-comment-actions.ts +++ b/components/video-page/hooks/use-comment-actions.ts @@ -61,6 +61,17 @@ function getAudioUploadFilename(blob: Blob): string { return 'recording.webm'; } +/** + * A step of the submit that failed with something worth reading out. + * + * The attachment goes up before the comment does, so a full account fails on the + * image and never reaches the comment at all. Reporting that as "failed to add + * comment" tells the uploader to try again, which is the one thing that cannot + * work. Carried as its own error type so a network fault, which has no message + * anybody wants to see, still falls back to the generic line. + */ +class CommentSubmitError extends Error {} + export function useCommentActions({ videoId, setVideo, @@ -262,7 +273,12 @@ export function useCommentActions({ body: imageFormData, }); - if (!imageRes.ok) throw new Error('Failed to upload image'); + if (!imageRes.ok) { + const imagePayload = (await imageRes.json().catch(() => null)) as { + error?: string; + } | null; + throw new CommentSubmitError(imagePayload?.error || 'Failed to upload image'); + } const imageDataResponse = await imageRes.json(); imageData = { url: imageDataResponse.data.url }; } @@ -320,9 +336,10 @@ export function useCommentActions({ ), }; }); - toast.error('Failed to add comment'); + const payload = (await res.json().catch(() => null)) as { error?: string } | null; + toast.error(payload?.error || 'Failed to add comment'); } - } catch { + } catch (error) { setVideo((prev) => { if (!prev) return prev; return { @@ -334,7 +351,7 @@ export function useCommentActions({ ), }; }); - toast.error('Failed to add comment'); + toast.error(error instanceof CommentSubmitError ? error.message : 'Failed to add comment'); } finally { setIsSubmittingComment(false); setIsUploadingImage(false); diff --git a/tests/component/hooks/use-comment-actions.test.ts b/tests/component/hooks/use-comment-actions.test.ts index e875b20..c3affab 100644 --- a/tests/component/hooks/use-comment-actions.test.ts +++ b/tests/component/hooks/use-comment-actions.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { useState } from 'react'; +import { useState, type ChangeEvent } from 'react'; import { act, renderHook, type RenderHookResult } from '@testing-library/react'; import { useCommentActions } from '@/components/video-page/hooks/use-comment-actions'; import type { Comment, CommentTag, VideoData } from '@/components/video-page/types'; @@ -293,6 +293,72 @@ describe('useCommentActions adding a comment', () => { expect(harness.result.current.actions.isSubmittingComment).toBe(false); }); + // The attachment goes up before the comment does, so a full account fails on + // the image and never reaches the comment at all. Reporting that as a comment + // that would not post told the uploader to try again, which is the one thing + // that cannot work. + it('reads out the storage error the attachment upload came back with', async () => { + fetchMock.mockImplementation((url: string) => { + if (url === '/api/upload/image') { + return Promise.resolve({ + ok: false, + status: 507, + json: () => + Promise.resolve({ + error: 'Storage limit exceeded. Please delete some files to free up space.', + }), + }); + } + return Promise.resolve(ok({ data: serverComment })); + }); + const harness = renderActions(); + + // A one-pixel PNG header is enough: the client only sniffs the magic bytes. + const png = new File( + [new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])], + 'n.png', + { + type: 'image/png', + } + ); + await act(async () => { + await harness.result.current.actions.handleImageSelect({ + target: { files: [png] }, + } as unknown as ChangeEvent); + }); + + act(() => harness.result.current.actions.setCommentText('Colour is off')); + await act(async () => { + await harness.result.current.actions.handleAddComment(); + }); + + expect(toastError).toHaveBeenCalledWith( + 'Storage limit exceeded. Please delete some files to free up space.' + ); + expect(commentIds(harness)).toEqual(['c1', 'c2']); + }); + + it('reads out the storage error the comment itself came back with', async () => { + fetchMock.mockResolvedValue({ + ok: false, + status: 507, + json: () => + Promise.resolve({ + error: 'Storage limit exceeded. Please delete some files to free up space.', + }), + }); + const harness = renderActions(); + + act(() => harness.result.current.actions.setCommentText('Colour is off')); + await act(async () => { + await harness.result.current.actions.handleAddComment(); + }); + + expect(toastError).toHaveBeenCalledWith( + 'Storage limit exceeded. Please delete some files to free up space.' + ); + }); + it('rolls the comment back out of the list when the request throws', async () => { fetchMock.mockRejectedValue(new Error('offline')); const harness = renderActions();