Stirling-PDF icon indicating copy to clipboard operation
Stirling-PDF copied to clipboard

refactor(exceptions): RFC 7807 Problem Details, ResourceBundle i18n, and error factory pattern

Open balazs-szucs opened this issue 1 month ago • 6 comments

Description of Changes

Implemented RFC 7807 problem-details standard to provide consistent, structured error responses across the application. All exceptions now flow through a centralized GlobalExceptionHandler with tailored handlers for password-protected files, corruption, validation, and Spring framework errors. Every response includes localized errorCode, contextual hints, actionable guidance, timestamps, and request paths.

Key improvements:

  • Unified error handling prevents inconsistent response formats across services
  • Localized error messages support multiple languages at runtime
  • Error context (hints, actionRequired field) improves user experience
  • Structured metadata enables programmatic error handling on client side

Reference: https://www.baeldung.com/spring-boot-return-errors-problemdetail

Exception Handling

  • Centralized all exception handling in GlobalExceptionHandler with specific handlers for password protection, file corruption, validation errors, and Spring exceptions
  • Refactored ExceptionUtils to generate consistent ProblemDetail responses with error metadata
  • Introduced ErrorCode enum to centralize error identification and metadata (supports localization)
  • Added helper methods for creating standardized exceptions across services

Implementation Details

  • Replaced generic exceptions with ExceptionUtils methods across utility classes (CbrUtils, CbzUtils, PdfToCbrUtils, EmlProcessingUtils, ImageProcessingUtils) for consistent error handling of invalid formats, empty files, and missing resources
  • Improved method signatures in JobExecutorService and AutoJobAspect by explicitly declaring thrown exceptions
  • Refined job execution error handling by removing unnecessary catch blocks in synchronous execution, allowing exceptions to propagate to the centralized handler
  • Enhanced error messages for unsupported formats (e.g., RAR5 limitations, PDF-to-CBR constraints) with clearer user guidance

Response Format

All errors now include:

  • errorCode: Machine-readable error identifier (localized at runtime)
  • title: Localized error title
  • detail: Context-specific error message
  • hints: Array of actionable suggestions
  • actionRequired: Boolean flag indicating if user action is needed
  • timestamp: Error occurrence time
  • path: Request path where error occurred

Error examples shown in PR include: corrupt files, invalid passwords, unsupported formats (RAR5), missing images, OOM conditions, and invalid HTML

Why This Change

  • Eliminates ad-hoc error handling patterns scattered across services
  • Provides actionable guidance instead of technical stack traces
  • Supports error messages in multiple languages without code changes
  • Aligns with RFC 7807, improving API interoperability
  • Error context reduces time to diagnose issues

Error sample (note the unformatted JSON is NOT visible by default but for presenting purposes I clicked on show stack on each error. By default only error banner (top part of error) visible)

Corrupt file: image Password-protected file (invalid password): image Unsupported RAR: image No images in RAR/ZIP: image

OOM: image Invalid html (on the html to pdf endpoint) image

GS conversion fail: image


Checklist

General

Documentation

UI Changes (if applicable)

  • [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR)

Testing (if applicable)

  • [ ] I have tested my changes locally. Refer to the Testing Guide for more details.

balazs-szucs avatar Oct 31 '25 14:10 balazs-szucs

/deploypr

Frooodle avatar Nov 07 '25 15:11 Frooodle

For controllers which dont throw a custom error but instead are just generic exceptions how do these show? Any generic error codes but still shwoing e.getMessage examples etc?

Frooodle avatar Nov 07 '25 15:11 Frooodle

For controllers which dont throw a custom error but instead are just generic exceptions how do these show? Any generic error codes but still shwoing e.getMessage examples etc?

Yes

In practice they look like this:

{
  "type": "/errors/io-error",
  "title": "File Processing Error",
  "status": 500,
  "detail": "Cannot run program \"unoconvert\": error=2, No such file or directory",
  "instance": "/api/v1/convert/file/pdf",
  "timestamp": "2025-11-07T16:06:59.592043239Z",
  "path": "/api/v1/convert/file/pdf",
  "hints": [
    "Confirm the file exists and is accessible.",
    "Ensure the file is not corrupted and is of a supported type.",
    "Retry the operation in case of transient I/O issues."
  ],
  "actionRequired": "Verify the file and try the request again."
}

Generally, these generic responses aren't completely generic since they extract e.getMessage() into the detail field. This is still some improvement over raw stack traces, but it's pretty broad after all, so for these it was more of "best effort" implementation from me, wasn't sure what to do in these cases besides that.

balazs-szucs avatar Nov 07 '25 16:11 balazs-szucs

image image

These are "good" examples, some controllers sadly completely lack any exception and error management in those cases it's tossup what would happen. Generally, I haven't spent that much hunting these down, so there might be some very edge cases.

balazs-szucs avatar Nov 07 '25 16:11 balazs-szucs

Although now that I think about it, we should throw a custom error for Cannot run program \"unoconvert\": error=2, No such file or directory as well, right? Right now, as the example shows above, it's one of the generic catch-all 500 errors, but I think it should have some custom hints, like "have you started unoserver? If not, you can start it with X" or "do you have unoserver installed?" etc. What do you say? (well, address in another PR, this is already a lot.)

Generally, to reiterate a bit better: where no error was thrown before, no error will be thrown now; where error 400 was thrown, it'll be replaced by the new error 400; where error 500 was thrown, the new error 500 will be thrown, etc. So there shouldn't be any regression from this.

balazs-szucs avatar Nov 07 '25 17:11 balazs-szucs