Skip to content

fix: register AppError handler, PDF validation, duplicate field crash, Pydantic V2, tests init, Dockerfile PYTHONPATH#318

Open
vidhip222 wants to merge 3 commits intofireform-core:mainfrom
vidhip222:main
Open

fix: register AppError handler, PDF validation, duplicate field crash, Pydantic V2, tests init, Dockerfile PYTHONPATH#318
vidhip222 wants to merge 3 commits intofireform-core:mainfrom
vidhip222:main

Conversation

@vidhip222
Copy link

Closes #311, #295, #293, #251, #235, #182, #137, #129, #116, #118

Description:
This PR fixes several bugs that were causing incorrect behavior across the API, data layer, and Docker environment. The fixes address unhandled exceptions returning 500 errors, silent data corruption in the database, a crash in the LLM response handler, Pydantic V2 deprecation warnings, missing test infrastructure, and a broken Docker PYTHONPATH.

Why is this needed?
Multiple routes were returning generic 500 errors instead of meaningful JSON error responses. The database was silently storing corrupt records when PDF generation failed. The LLM field handler would crash on duplicate fields. The Docker container's PYTHONPATH excluded the api package entirely, breaking all imports in a deployed environment.

Key Changes:

  • AppError Handler Registration: register_exception_handlers(app) is now called in api/main.py before routers are included, so all AppError exceptions return proper JSON responses with the correct HTTP status codes instead of 500s.
  • PDF File Validation: file_manipulator.py now raises FileNotFoundError instead of silently returning None when a PDF template doesn't exist on disk. The forms route catches this and raises an AppError(404), preventing corrupt FormSubmission records from being written to the database.
  • Duplicate Field Crash Fix: add_response_to_json() in llm.py now safely converts a scalar value to a list before appending when the same field is processed more than once.
  • Pydantic V2 Compatibility: Replaced the deprecated class Config: from_attributes = True pattern with model_config = ConfigDict(from_attributes=True) in FormFillResponse and TemplateResponse.
  • Test Package Init: Added missing tests/init.py so pytest can discover and run tests correctly.
  • Dockerfile PYTHONPATH: Fixed PYTHONPATH=/app/src → PYTHONPATH=/app so that api.* imports resolve correctly inside the container.

Type of Change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • AppError handler: Send a POST to /forms/fill with a non-existent template_id. Expect 404 JSON response instead of 500.

Stale template / missing PDF: Send a POST to /forms/fill with a valid template_id pointing to a deleted PDF file. Expect 404 JSON response and no new FormSubmission record in the database.

Duplicate field crash: Process a transcript where the LLM returns a value for the same field more than once. No crash, values are collected into a list.

Pydantic warnings: Start the API server and confirm no PydanticDeprecatedSince20 warnings appear in the logs.

Tests: Run pytest tests/ from the project root. Tests are discovered and run without import errors.

Docker: Build and run the Docker container. Api.* imports resolve and the server starts correctly.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

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.

[BUG]: AppError handler is never registered

1 participant