Propagate asynchronous errors during server-side rendering to a client

[Platonn]

Which @angular/* package(s) are relevant/related to the feature request?

platform-server

Description

Problem:

By default ExpressJS always returns a "success" response (with an HTTP status of 200) containing the HTML that was eventually rendered, regardless of any asynchronous errors that may have occurred during the rendering of an Angular app, e.g. HTTP errors from backend endpoints or runtime errors in your components rendered asynchronously.

This behavior might be confusing for end-users and can be also a problem for search engine optimization (SEO), because it can result in Google indexing malformed HTML. This is because HTTP errors from backend endpoints may cause missing data to be displayed in the final HTML output, and runtime errors in a component's logic may cause the HTML to be rendered incorrectly.

When ExpressJS receives a request from a client, it calls the ngExpressEngine from the public API of @nguniversal/express-engine that under the hood uses the method renderModule() from @angular/platform-server to render an app and obtain the final HTML output. This last function returns a Promise that either resolves to the final HTML string output or rejects with the error payload. As of Angular 15, this Promise is rejected only in 2 cases:

• when an error happens in some of Angular APP_INITIALIZER hooks
• during the synchronous bootstrap of the root component.

The implementation of the Angular's function renderModule() doesn't allow for rejecting the Promise in any other case, e.g. when an error happens in some other asynchronous task during the rendering (e.g. in a child component that is rendered after a delay, or after making some http calls to a backend). For reference, you might check the Angular v15.2 implementation of the function "renderModule()" and it's dependency function "_render()".

I believe this problem is important for all users of the Angular rendering on the server. I think Angular by default should a safety mechanism to prevent SEO degradation when some async error happens during the rendering (e.g. when a backend API is temporarily unavailable or when a runtime error occurs in the application).

Proposed solution

I can see 2 possible solutions: a) the Promise should be rejected immediately, whenever an error reaches ErrorHandler b) the Promise should be rejected only when the app is stable, after an error reaches ErrorHandler

Note: You might want to recover from some errors successfully in your custom ErrorHandler, to only errors re-thrown from ErrorHandler should be taken into consideration.

Alternatives considered

As a workaround for now, we can build a custom mechanism of propagating errors directly from Angular's ErrorHandler to the ExpressJS server (e.g. via a custom callback provided into Angular's Dependency Injection) and then ignore the result of renderModule() (and ngExpressEngine). Example draft implementation below:

Angular app - define injection token PROPAGATE_ERROR_TO_CLIENT, and use it in custom ErrorHandler

import { ErrorHandler } from `@angular/core`;

export const PROPAGATE_ERROR_TO_CLIENT = new InjectionToken<(error:unknown)=>void>('PROPAGATE_ERROR_TO_CLIENT');

@Injectable()
export class CustomErrorHandler extends ErrorHandler {
   propagateErrorToClient = inject(PROPAGATE_ERROR_TO_CLIENT);
   handleError(error){
      this.propagateErrorToClient(error)
   }
}

Express app - provide a concrete function for injection token PROPAGATE_ERROR_TO_CLIENT that send the given error to a client.

import { format } from 'node:util';

server.get('*', (req, res) => {
  function propagateErrorToClient(error) {
    res.status(500).send(format(error));
  }
  
  res.render(indexHtml, {
    req,
    providers: [{ provide: PROPAGATE_ERROR_TO_CLIENT, useValue: propagateErrorToClient }],
    (err,html) => {
      if(res.headersSent) { return; } // avoid sending response, if we already did it (e.g. if `propagateErrorToClient()` was invoked before)
      if(err) { res.status(500).send(format(err)); return; }
      res.status(200).send(html);
    }
  });
});

Pros:

• it works :D

Cons:

• it tightly couples the ExpressJS app with the internals of Angular app via a new injection token PROPAGATE_ERROR_TO_CLIENT
• when error is propagated to a client, the app is yet not stable and rendering not finished. Eventually when the rendering is finished, we have to explicitly ignore the result of the rendering, if a response has been already sent to a client (because propagateErrorToClient was called before).
• we work around the standard API of Express rendering engine (ngExpressEngine and renderModule from Angular), which returns possible 2 types of results: HTML or error, when the rendering is finished.

---

Here's another custom alternative approach that @eneajaho took in his library ngx-isr (btw. kudos to Enea for your work 🙌 ) took a different approach:

• intercept all http errors and collect them to an array. Note: ngx-isr doesn't collect other asynchronous errors. But this could be easily added by providing a custom ErrorHandler.
• in the hook BEFORE_APP_SERIALIZED, the errors are embedded into the final HTML as a <script> with JSON - similar technique to the Angular's "TrasferState" (but here it's used to transfer data not to the client, but to the server middleware)
• then in the server middleware, the final HTML is parsed so to extract possible errors from the <script> appended in the end of the html body and if it contains embedded errors, the final HTML is ignored

Observation: this approach waits until the app is stable (no async tasks).

[alan-agius4]

Related to https://github.com/angular/angular/issues/33642.

Thanks for the request.

I do think that we need to better handle this case, but this would require some flashing out.

Returning 500 will cause application to be rendered completely, this was previously something that we deemed bad users experience as a better UX would to re-retry the request when the application bootstrapped on the client side.

Unless I am missing something, from what I am understanding, ngx-isr does not do anything different when there is an error in terms of response. It just do not store the response in the cache.

[Platonn]

Hi @alan-agius4, thanks for linking this issue to a related one. Now I tried simply re-throwing an error from my custom ErrorHandler, as somebody advised in the comment https://github.com/angular/angular/issues/33642#issuecomment-557199699 . Results:

• then the function renderModule() rejects a promise, not only when an error occurs in the constructor of AppComponent, but even when an error occurs synchronously in the ngOnInit(). In other words, error occuring in the first synchronous change detection cycle of Angular components (i.e. in the first tick after attaching the root component) is propagated to a client via a standard rejection of the renderModule() promise.
• but errors happening asynchronously are still not propagated properly. Even worse, the whole NodeJS process with ExpressJS app exits with status 1, when I re-throw an asynchronous error from my custom ErrorHandler.

Repo for reproduction: https://github.com/Platonn/angular-ssr-error-handling__ng16-1

[Platonn]

Returning 500 will cause application to be rendered completely, this was previously something that we deemed bad users experience as a better UX would to re-retry the request when the application bootstrapped on the client side.

Excuse me, maybe my original ticket description was misleading a bit.

The goal of this feature request is not propagating the error to the end-user client, but propagating it in general to an ExpressJS middleware, where it can be handled there and return an appropriate user-friendly response to the end user and Google crawler. (e.g. we could send a "CSR fallback" with just plain scaffold index.html and possibly 500 http status or "Cache-control: no-store" in order to avoid such a response to be cached/indexed by the user/CDN/crawler).

The point of this ticket is: let's propagate asynchronous errors from renderModule(). Currently they are swallowed and ExpressJS doesn't have a chance to know about them, in order to send an appropriate response to the end-user 😉

[alan-agius4]

The point of this ticket is: let's propagate asynchronous errors from renderModule(). Currently they are swallowed and ExpressJS doesn't have a chance to know about them, in order to send an appropriate response to the end-user 😉

In this case the request seems like a duplicate of https://github.com/angular/angular/issues/33642 right? IE: errors should be returned in form of diagnostics or cause the promise to be rejected when calling renderModule.

[Platonn]

Agreed. Duplicate.

I have problem to understand what did you mean by "returned in form of diagnostics?". Could you give an example?

[alan-agius4]

Example, the return of renderModule would be changed to something like {content: “…”, errors: [..], …}

Closing in favor of https://github.com/angular/angular/issues/33642

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}