Skip to content

fix: async error handling in view rendering #6486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Abdel-Monaam-Aouini
Copy link
Contributor

PR Description: Fix async error handling in view rendering
This PR addresses an issue with error handling in the tryRender function related to async/await operations. The original implementation only properly handled synchronous errors, while errors occurring asynchronously in the view.render process could be missed.
The fix properly wraps the callback with explicit error handling to ensure both sync and async errors are caught and properly propagated to the calling function.
Changes:

  1. Modified the tryRender function to explicitly handle errors from the view render callback
  2. Ensures proper error propagation in asynchronous code paths
  3. Maintains the existing synchronous error handling capability

@Abdel-Monaam-Aouini Abdel-Monaam-Aouini changed the title fix: enhance error handling in view rendering callback fix: async error handling in view rendering Apr 28, 2025
@krzysdz
Copy link
Contributor

krzysdz commented May 3, 2025

This PR addresses an issue with error handling in the tryRender function related to async/await operations.

I don't see any change here that has anything to do with async functions or Promises. What is the problem that you're trying to solve (code example would be best)? Your tests pass even without this change (because it seems to do exactly nothing) and lib/application.js already has 100% line coverage.

It looks like your change is more or less view.render(options, callback) => view.render(options, (err, result) => callback(err, result)) and only introduces a small overhead.

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.

2 participants