Skip to content

fix: prevent UI from hanging on unexpected errors during capture flow#1464

Open
flacoonb wants to merge 2 commits intoPhotoboothProject:devfrom
flacoonb:fix/reload-error
Open

fix: prevent UI from hanging on unexpected errors during capture flow#1464
flacoonb wants to merge 2 commits intoPhotoboothProject:devfrom
flacoonb:fix/reload-error

Conversation

@flacoonb
Copy link
Contributor

Add comprehensive try/catch error handling to all critical paths in the photo capture flow. Previously, only .fail() callbacks had error handling while .done() callbacks and other functions could silently crash, leaving the UI stuck on the loader screen with takingPic=true.

Changes:

  • Wrap errorPic cleanup in try/catch so page reload always fires
  • Add fallback timeout (5s) if notificationTimeout is NaN
  • Add try/catch to thrill(), takePic(), callTakePicApi .done(), callTakeVideoApi .done(), processPic success, processVideo success
  • Add missing timeout (25s) to callTakeVideoApi jQuery post

Prerequisites checklist

What is the purpose of this pull request? (put an "x" next to an item)

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (Give an overview)

When config.dev.reload_on_error is enabled, the photobooth should automatically reload the page after displaying an error. However, in certain error scenarios the UI would get stuck on the error/loader screen and never reload.

Root cause: All critical functions in the capture flow (thrill(), takePic(), errorPic(), .done() callbacks, processPic, processVideo) execute inside setTimeout or jQuery callbacks with no try/catch. If any exception occurs (e.g. from remoteBuzzerClient, photoboothTools.getTranslation(), or DOM manipulation), the callback silently aborts and the reload logic never fires. The takingPic flag stays true, effectively locking the UI.

Fix: Added try/catch blocks around all critical paths so that:

  1. errorPic() always reaches the reload logic, even if cleanup code throws
  2. Any unexpected error in thrill(), takePic(), or .done() callbacks is caught and routed to errorPic() for proper error display and reload
  3. callTakeVideoApi now has a 25s timeout to prevent hanging on unresponsive backends
  4. A fallback timeout of 5s is used if notificationTimeout is NaN

Additionally, the CI workflow triggers were extended to also run on fix/** and feature/** branches.

Is there anything you'd like reviewers to focus on?

  • The try/catch blocks in errorPic(): the reload logic is intentionally placed outside the try block so it always executes, even if cleanup fails.
  • The catch {} (parameter-less catch) syntax is used where the error object is not needed, to satisfy ESLint's no-unused-vars rule.
  • The notificationTimeout || 5000 fallback in errorPic() ensures reload works even if the timeout config is misconfigured.

AI used to create this Pull Request?

Yes, Claude Code (claude-sonnet-4-6) was used to assist with:

  • Analyzing the error handling flow to identify the root cause
  • Implementing the try/catch blocks across all critical paths
  • Running local CI checks (ESLint, PHPStan, PHPUnit, gulp build, pre-commit hooks)

Add comprehensive try/catch error handling to all critical paths in the
photo capture flow. Previously, only .fail() callbacks had error handling
while .done() callbacks and other functions could silently crash, leaving
the UI stuck on the loader screen with takingPic=true.

Changes:
- Wrap errorPic cleanup in try/catch so page reload always fires
- Add fallback timeout (5s) if notificationTimeout is NaN
- Add try/catch to thrill(), takePic(), callTakePicApi .done(),
  callTakeVideoApi .done(), processPic success, processVideo success
- Add missing timeout (25s) to callTakeVideoApi jQuery post
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