Skip to content

fix: show error feedback in Import Map dialog and standardize API error responses#333

Merged
fank merged 2 commits intomainfrom
fix/import-map-error-handling
Mar 8, 2026
Merged

fix: show error feedback in Import Map dialog and standardize API error responses#333
fank merged 2 commits intomainfrom
fix/import-map-error-handling

Conversation

@fank
Copy link
Member

@fank fank commented Mar 8, 2026

Summary

  • Import Map dialog now displays errors inline instead of silently failing — covers server offline, proxy 413 (body too large), bad zip, and invalid grad_meh structure
  • Backend: converted importMapToolZip from raw http.HandlerFunc to idiomatic fuego handler (ContextNoBody signature), removed unused writeJSONError helper
  • All API endpoints: added Detail field to fuego error returns — previously only Err was set, which is json:"-" and never reached the client

Test plan

  • Go tests pass (go test ./internal/server/)
  • UI type-check passes (npx tsc --noEmit)
  • UI dialog tests pass (npx vitest run dialogs.test.tsx)
  • Manual: upload a zip with server offline → error banner shows "Import failed: Upload network error"
  • Manual: upload a large file behind nginx with low client_max_body_size → error banner shows proxy upload limit message
  • Manual: upload an invalid zip → error banner shows extraction/validation error from server
  • Manual: successful upload still works as before

…or responses

The Import Map dialog silently swallowed errors (server offline, proxy 413,
bad zip) — the progress bar would just disappear with no feedback. Now errors
are displayed inline in the dialog footer.

Backend: convert importMapToolZip from raw http.HandlerFunc to idiomatic fuego
handler. Add Detail field to all fuego error returns across all endpoints so
error messages are serialized to the client (Err is json:"-" and was invisible).

Frontend: parse fuego's detail field in XHR error responses, display error
banner in ImportDialog with specific message for 413 and generic fallback.
@github-actions
Copy link

github-actions bot commented Mar 8, 2026

Coverage Report for ui

Status Category Percentage Covered / Total
🔵 Lines 99.03%
⬇️ -0.04%
4839 / 4886
🔵 Statements 98.44%
⬇️ -0.06%
6730 / 6836
🔵 Functions 97.83%
🟰 ±0%
1899 / 1941
🔵 Branches 89.16%
⬇️ -0.21%
2223 / 2493
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
ui/src/data/apiClient.ts 97.43%
⬇️ -1.89%
92.1%
⬇️ -5.12%
97.72%
🟰 ±0%
98.63%
⬇️ -0.66%
476, 525-526
ui/src/i18n/locales.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
ui/src/pages/map-manager/MapManager.tsx 97.74%
⬇️ -0.32%
82.14%
⬇️ -2.47%
97.95%
⬆️ +0.02%
97.8%
⬇️ -0.48%
52, 62, 83, 126, 136, 160
ui/src/pages/map-manager/dialogs.tsx 98.59%
⬆️ +0.06%
91.66%
🟰 ±0%
95.89%
⬆️ +0.24%
100%
🟰 ±0%
52, 69, 99
Generated in workflow #424 for commit 416f4c7 by the Vitest Coverage Report Action

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the user experience for importing maps by providing clear, immediate error feedback directly within the UI. Concurrently, it standardizes the backend's API error responses to include more descriptive details, ensuring that the UI can present meaningful messages for various failure conditions, from network issues to invalid file formats. This change improves debuggability and user guidance without altering core functionality.

Highlights

  • Improved Import Map Dialog Error Feedback: The Import Map dialog in the UI now displays detailed error messages inline for various failure scenarios, including server offline, proxy body too large (413), invalid zip files, and incorrect grad_meh structure. This replaces previous silent failures.
  • Backend API Error Standardization: All backend API endpoints using the fuego framework now include a Detail field in their error responses. This provides more specific and user-friendly error information to clients, which was previously only available in the internal Err field.
  • Refactored importMapToolZip Handler: The importMapToolZip backend handler was refactored from a raw http.HandlerFunc to an idiomatic fuego handler with a ContextNoBody signature, aligning it with other API endpoints and simplifying error handling.
Changelog
  • internal/server/handler.go
    • Updated the importMapToolZip route to use fuego.Post instead of fuego.PostStd.
    • Added the Detail field to fuego.NotFoundError responses.
  • internal/server/handler_admin.go
    • Added the Detail field to fuego.BadRequestError responses for parsing errors and invalid focus fields.
    • Added the Detail field to fuego.NotFoundError responses.
    • Added the Detail field to fuego.ConflictError responses.
  • internal/server/handler_blacklist.go
    • Added the Detail field to fuego.BadRequestError responses for invalid ID or player ID parameters.
  • internal/server/handler_maptool.go
    • Added the Detail field to fuego.InternalServerError responses for map scanning errors.
    • Added the Detail field to fuego.BadRequestError responses for invalid map names.
    • Converted the importMapToolZip function from an http.HandlerFunc to a fuego handler with ContextNoBody signature.
    • Modified importMapToolZip to return fuego errors with Detail fields for various upload and processing failures (e.g., missing file, invalid zip, extraction errors, invalid grad_meh structure).
    • Removed the unused writeJSONError helper function.
    • Added the Detail field to fuego.InternalServerError responses for restyle errors.
    • Added the Detail field to fuego.BadRequestError responses when no maps are found for restyling.
    • Added the Detail field to fuego.BadRequestError responses for job cancellation errors.
  • internal/server/handler_maptool_test.go
    • Removed encoding/json import as it's no longer needed for error parsing.
    • Updated TestImportMapToolZip_NotZip to use fuego.NewMockContextNoBody and assert on fuego.BadRequestError type and error message.
    • Updated TestImportMapToolZip_MissingFile to use fuego.NewMockContextNoBody and assert on fuego.BadRequestError type and error message.
    • Updated TestImportMapToolZip_ValidZip to use fuego.NewMockContextNoBody and assert on successful return.
    • Updated TestImportMapToolZip_BadExtract to use fuego.NewMockContextNoBody and assert on fuego.BadRequestError type and error message.
    • Updated TestImportMapToolZip_NoGradMehDir to use fuego.NewMockContextNoBody and assert on fuego.BadRequestError type and error message.
    • Removed the jsonTrimmed helper function.
  • ui/src/data/apiClient.ts
    • Modified the importMapToolZip error handling to parse the response body for detail or error fields to provide more specific error messages.
  • ui/src/i18n/locales.ts
    • Added new translation keys mm_upload_failed and mm_upload_too_large for localized error messages.
  • ui/src/pages/map-manager/MapManager.tsx
    • Added a new state variable uploadError to store and display import errors.
    • Updated handleImport to reset uploadError at the start of an import.
    • Implemented error handling in handleImport to catch API errors, specifically checking for HTTP 413 (Payload Too Large) to display a dedicated message, and general import failures.
    • Passed the uploadError state to the ImportDialog component.
    • Modified onClose handler for ImportDialog to clear uploadError.
  • ui/src/pages/map-manager/tests/dialogs.test.tsx
    • Updated all ImportDialog test renderings to include the new uploadError prop, passing null by default.
  • ui/src/pages/map-manager/dialogs.module.css
    • Added new CSS styles for the .uploadError class to visually present error messages in the dialog footer.
  • ui/src/pages/map-manager/dialogs.tsx
    • Added uploadError as a prop to the ImportDialog component.
    • Implemented a Show component to conditionally render an error banner in the dialog footer when props.uploadError is present, including an AlertTriangleIcon.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the importMapToolZip handler to use idiomatic fuego patterns and standardizes API error responses by adding a Detail field, aiming to provide clearer user feedback. However, these changes introduce a systemic security risk by exposing internal error details to the client. A potential access control flaw was also identified in the MapTool event stream.

…d info leaks

InternalServerError and NotFoundError now use static, descriptive detail
messages instead of err.Error() which could expose file paths or DB internals.
BadRequestError keeps err.Error() since those are input validation messages
(strconv parse errors, JSON decode errors) that help the user fix their request.
@github-actions
Copy link

github-actions bot commented Mar 8, 2026

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/OCAP2/web/internal/server 94.52% (+0.37%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/OCAP2/web/internal/server/handler.go 96.31% (ø) 271 261 10
github.com/OCAP2/web/internal/server/handler_admin.go 100.00% (ø) 99 99 0
github.com/OCAP2/web/internal/server/handler_blacklist.go 100.00% (ø) 28 28 0
github.com/OCAP2/web/internal/server/handler_maptool.go 91.47% (+2.82%) 129 (-12) 118 (-7) 11 (-5) 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/OCAP2/web/internal/server/handler_maptool_test.go

@fank fank merged commit 530db1a into main Mar 8, 2026
3 checks passed
@fank fank deleted the fix/import-map-error-handling branch March 8, 2026 10:38
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.

1 participant