Skip to content

Improve PFP upload error messaging#1753

Merged
simo6529 merged 5 commits intomainfrom
simo6529/pfp-upload-investigate
Jan 19, 2026
Merged

Improve PFP upload error messaging#1753
simo6529 merged 5 commits intomainfrom
simo6529/pfp-upload-investigate

Conversation

@simo6529
Copy link
Copy Markdown
Collaborator

@simo6529 simo6529 commented Jan 19, 2026

Adds a shared upload error normalizer so uploads surface friendly network/403/size failures. Wires the PFP edit flow to use it instead of raw errors. No behavioral changes beyond clearer toasts.

Summary by CodeRabbit

  • Bug Fixes

    • Improved upload error messages: more specific, user-friendly guidance for network failures, permission denials, and oversized files.
  • Chores

    • Upgraded core web framework and React versions for better compatibility and performance.
    • Improved workspace setup and sync tooling to standardize editor settings and pre-commit behavior for contributors.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Simo <simo@6529.io>
Signed-off-by: Simo <simo@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 19, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds a centralized upload-error interpreter and wires it into the profile-picture upload UI; upgrades Next/React versions; and refactors and extends worktree tooling with shared helpers, VS Code settings, pre-commit hook setup, improved symlink creation, and sync.conf changes.

Changes

Cohort / File(s) Summary
Upload error utility
services/api/upload-error.ts
New module getUploadErrorMessage(error: unknown) plus helpers to classify Axios and generic upload errors into user-facing hints.
Component integration
components/user/user-page-header/pfp/UserPageHeaderEditPfp.tsx
Use getUploadErrorMessage(error) in updatePfp mutation onError to produce toast messages (replaced raw error-to-string cast).
Worktree tooling (scripts)
scripts/worktree/wt-common.sh, scripts/worktree/wt-add.sh, scripts/worktree/wt-sync.sh, scripts/worktree/sync.conf
Added helper functions setup_vscode_settings and setup_precommit_hook; refactored wt-add to source helpers and delegate VSCode/hook setup; wt-sync now ensures source dirs, computes robust relative symlinks (Python-assisted fallback), skips main repo, and runs post-sync VSCode/hook setup; sync.conf entries changed to directory form and .agent/ added.
Next config
config/nextConfig.ts
Removed custom webpack config and node:path import; left turbopack alias stubs.
Dependencies
package.json
Bumped next to 16.1 and loosened react / react-dom to ^19.2.3.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer CLI
  participant Sync as wt-sync.sh
  participant Main as Main Repo (fs)
  participant WT as Worktree (fs)
  participant Python as Python (optional)
  participant Git as Git
  participant VSCode as .vscode/settings.json
  participant Hooks as .hooks/pre-commit

  Dev->>Sync: invoke wt-sync.sh (worktree)
  Sync->>Main: read sync.conf
  Sync->>Main: ensure_source_dirs()
  Main-->>Sync: create/repair missing source dirs
  Sync->>WT: for each mapping, create_symlink(rel_path)
  Sync->>Python: request relative symlink path (if available)
  alt Python available
    Python-->>Sync: computed relative path
  else
    Sync-->>Sync: fallback path computation or error
  end
  Sync->>Git: configure worktree hooks (core.hooksPath, extensions.worktreeConfig)
  Sync->>VSCode: setup_vscode_settings(worktree_path)
  Sync->>Hooks: setup_precommit_hook(worktree_path)
  Sync->>WT: run post-sync npm install (subshell)
  Sync-->>Dev: finish
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • prxt6529
  • ragnep

Poem

🐰
I nibbled errors into kinder lines,
Tuned links and hooks and tiny signs,
Painted VSCode with a gentle hue,
Synced every path so it’s good as new,
Hoppity — the repo’s trimmed for you! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly matches the main objective: adding a shared upload error normalizer for clearer error messages in the PFP upload flow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@services/api/upload-error.ts`:
- Around line 31-36: The serverMsg assignment can end up being a non-string
(e.g., an object) causing toasts to show “[object Object]”; in the block that
builds serverMsg (using error.response?.data?.error and messageFromUnknown) add
a type check so you only use error.response.data.error when it's a string,
otherwise fall back to messageFromUnknown or a safe stringified version (e.g.,
JSON.stringify with try/catch) and finally ensure the function returns a string
by coalescing to FALLBACK_HINT if needed; update the logic around serverMsg and
the return so serverMsg is always a string before returning.

Comment thread services/api/upload-error.ts Outdated
Signed-off-by: Simo <simo@6529.io>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 19, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
4 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@package.json`:
- Line 112: Update the pinned dependency for eslint-config-next to match the
Next.js 16.1 upgrade: change the package.json entry for "eslint-config-next"
from "16.0.7" to a 16.1.x release (e.g. "16.1.1") so it aligns with the "next":
"16.1" entry; after editing the "eslint-config-next" version, run your package
install (npm/yarn/pnpm) to refresh the lockfile and verify linting passes.

In `@scripts/worktree/wt-common.sh`:
- Around line 88-92: The heredoc that emits the tailwind setting containing
backticks causes shell command substitution; update the heredoc usage so
backticks are not interpreted by the shell—either switch to a quoted heredoc
(e.g., use <<'EOF') or escape backticks inside the
tailwindCSS.experimental.classRegex block so the patterns like
"['\"`]([^'\"`]*tw-[^'\"`]*)['\"`]" are preserved; modify the section that
writes tailwindCSS.experimental.classRegex (the heredoc that includes clsx(),
cn(), cva() patterns) to use a safe quoted heredoc or escape each backtick and
ensure any injected variables (e.g., $color) are handled separately.

In `@scripts/worktree/wt-sync.sh`:
- Around line 125-149: The ensure_source_dirs function is performing destructive
operations (rm -f and mkdir -p) even in dry-run mode; add a guard at the top of
ensure_source_dirs to detect the dry-run flag (e.g., DRY_RUN or the existing
--dry-run variable used elsewhere) and return early with a log message when
dry-run is active, and similarly wrap the other blocks that call rm -f
"$source_path" and mkdir -p "$source_path" (the spots you noted around the other
removals/creates) with the same dry-run check so no filesystem mutations occur
during --dry-run.
- Around line 283-287: The string comparison between "$worktree_path" and
"$MAIN_REPO" can miss symlinked/realpath variants; resolve both to canonical
paths before comparing (e.g., use realpath or readlink -f) and compare those
values instead. Update the check around worktree_path and MAIN_REPO so it
computes resolved_worktree="$(realpath "$worktree_path" 2>/dev/null || readlink
-f "$worktree_path" 2>/dev/null || echo "$worktree_path")" and
resolved_main="$(realpath "$MAIN_REPO" 2>/dev/null || readlink -f "$MAIN_REPO"
2>/dev/null || echo "$MAIN_REPO")" and then compare resolved_worktree ==
resolved_main to decide to log_verbose "  Skipping main repo" and return.
🧹 Nitpick comments (2)
package.json (1)

119-122: Align React package ranges with pinned type packages.

react/react-dom use caret ranges (^19.2.3) while @types/react and @types/react-dom are pinned to exact versions (19.2.3). If a newer React 19.x version is released and resolved, the type definitions will not update automatically, potentially causing type/runtime mismatches. Consider pinning React to exact versions or loosening the type packages to match the same range.

scripts/worktree/wt-add.sh (1)

41-48: Consider removing duplicate VS Code + hook setup after wt-sync.

wt-sync.sh now runs setup_vscode_settings and setup_precommit_hook; doing it again here is redundant and can be trimmed.

Comment thread package.json
Comment thread scripts/worktree/wt-common.sh
Comment thread scripts/worktree/wt-sync.sh
Comment thread scripts/worktree/wt-sync.sh
@simo6529 simo6529 merged commit f6e3296 into main Jan 19, 2026
6 of 7 checks passed
@simo6529 simo6529 deleted the simo6529/pfp-upload-investigate branch January 19, 2026 08:59
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.

3 participants