Skip to content
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

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented May 14, 2024

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 alan-agius4 added area: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate labels May 14, 2024
@ngbot ngbot bot added this to the Backlog milestone May 14, 2024
@alan-agius4 alan-agius4 added the area: server Issues related to server-side rendering label May 14, 2024
@alan-agius4 alan-agius4 marked this pull request as ready for review May 14, 2024 06:57
@alan-agius4 alan-agius4 marked this pull request as draft May 14, 2024 08:19
@alan-agius4 alan-agius4 changed the title fix(core): propagate synchronous errors from lifecycle hooks when utilizing platform-server fix(platform-server): propagate errors from lifecycle hooks when utilizing platform-server May 14, 2024
@alan-agius4 alan-agius4 force-pushed the render-error-server branch 11 times, most recently from f4be47b to 88b383e Compare May 15, 2024 08:06
…izing platform-server

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 angular#33642
@pawelfras
Copy link

pawelfras commented May 20, 2024

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!

@AndrewKushnir AndrewKushnir added target: patch This PR is targeted for the next patch release and removed target: rc This PR is targeted for the next release-candidate labels May 22, 2024
@Platonn
Copy link
Contributor

Platonn commented May 31, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime area: server Issues related to server-side rendering target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

renderModuleFactory does not reject when error is thrown in application
4 participants