Skip to content

Bugfix/clean up failed note uploads - #246

Open
SXT230118 wants to merge 4 commits into
developfrom
bugfix/clean-up-failed-note-uploads
Open

SXT230118 wants to merge 4 commits into
developfrom
bugfix/clean-up-failed-note-uploads

Conversation

@SXT230118

Copy link
Copy Markdown
Contributor

Description

Attempt at resolving #204
Note: I saw some issues that could lead to an entry without a PDF being able to be a note, but I wasn't able to actually get that result, so maybe I was testing incorrectly or reading the code wrong (createFileFormSchema in note.ts).

Testing

Once logged in, go to create a note and make it fail somehow. I put the line below

throw new Error('TESTING: force upload failure');

in useUploadToUploadURLS.ts, right after const blob [...] and before const uploadResponse [...] because I couldn't figure out another way to fail it.

Once saving that file, try uploading a note. It will fail. Run

npx drizzle-kit studio

in your terminal to look at all the DB entries. Sort/filter by upload or update timing (descending), and you should NOT see an entry for your failed upload!

AI Disclosure

I used information from the Copilot search result that comes up in Edge to vaguely understand how nanoid works, how to generate an id for a new note before creating a DB row, and to understand some of the TypeScript lines in storage.ts. The VSCode AI autocomplete was partially used when rewriting the ownedFileProcedure in storage.ts (mostly copying the existing method with minor changes).

Checklist

  • Create this PR
  • Perform final self-review

@SXT230118
SXT230118 requested review from a team as code owners September 23, 2026 07:17
@vercel

vercel Bot commented Sep 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
utd-notebook Ready Ready Preview Sep 23, 2026 7:17am UTC

Request Review

@SXT230118 SXT230118 self-assigned this Sep 23, 2026
@SXT230118 SXT230118 linked an issue Sep 23, 2026 that may be closed by this pull request

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since id originates in a "use client" component and the create procedure accepts it directly, the database's nanoid(20) default is bypassed whenever an ID is provided. That means a client can submit an arbitrary ID instead of the NanoID generated by the UI. This ID should be generated server-side rather than accepted from the client.

This branch was successfully deployed

1 active deployment
Preview — 62a1c070 Deployed Sep 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up failed note uploads

2 participants