angular icon indicating copy to clipboard operation
angular copied to clipboard

fix(platform-server): propagate errors from lifecycle hooks when utilizing platform-server

Open alan-agius4 opened this issue 1 year ago • 1 comments

In previous versions, if an error occurred within the lifecycle hooks, the promises for renderModule and renderApplication were not rejected. This could result in serving a "broken" page during Server-Side Rendering (SSR), or the build erroneously being marked as successful during Static Site Generation (SSG).

Fixes #33642

alan-agius4 avatar May 14 '24 06:05 alan-agius4

Hey @alan-agius4, I appreciate your work and the fact that the Angular Core team has started working on Error handling in SSR/SSG! I would like to ask about two topics:

  • It may be beyond the scope of this PR, but how to trade HTTP errors from the backend? Make a custom HTTP interceptor that catches all backend HTTP errors and then calls ErrorHandler.handleError(HTTP error response). Such a kind of errors should also be handled so that in the end the user doesn't receive with high certainty malformed HTML with wrong status 200
  • What is the advantage of emitting a rejection of the promise whenStableWithoutError immediately, not only when the app is stable? In such a scenario, HTTP requests may still exist as 'stabilisation' is in progress. In both SSR and SSG.

I hope you don't mind that I added this comment directly to this PR.

Best regards!

pawelfras avatar May 20 '24 10:05 pawelfras

Hi @alan-agius4

Did you consider making the ErrorInterceptor public and extendable? Our use case is to capture the application's state (e.g. get data from the ngrx store via DI, or any other context from DI) and attach it as a context to the propagated error. Thanks to this context, we'll be able to better handle this error in the ExpressJS layer, where the Angular DI context is unavailable. For example, based on the additional context attached to the propagated error object, in ExpressJS we can set an appropriate HTTP error status code for the response to the client, e.g. 404 vs 500.

The BEFORE_APP_SERIALIZED is the hook of the "last moment" allowing to access Angular DI and modify the final HTML markup (e.g. attaching the TransferState).

Analogically, we need the hook of "last moment" allowing to access DI and modify the final error (or returning a brand new error object - use case: return a custom "wrapper Error object" with the cause property pointing to the original error)

If you don't want to expose in the public API the whole ErrorInterceptor, we could at least expose the injection token hook in @angular/platform-server, e.g. new InjectionToken<(error: unknown) => unknown>, maybe named as BEFORE_ERROR_PROPAGATED.

Platonn avatar May 31 '24 14:05 Platonn

Hi @alan-agius4,

Thank you so much for all the hard work you put into improving Angular SSR in various fields. And thank you for your efforts on this PR. I truly believe Async Error Handling is a significant improvement for Angular SSR/Prerendering.

I noticed the PR was recently closed, and I understand that there might be higher priorities on the Angular team's roadmap or perhaps a different approach might be in the works.

If it's a matter of having too many other duties, I'd be more than happy to help and contribute in any way I can. Please feel free to let me know how we could possibly collaborate to make Angular SSR Async Error Handling happen 😊

Thanks again!

Platonn avatar Oct 01 '24 07:10 Platonn