feat: CXSPA-6578 Introduce PROPAGATE_SERVER_ERROR_RESPONSE and default ExpressJS handlers #18753

pawelfras · 2024-04-22T10:55:52Z

This PR introduces the PROPAGATE_SERVER_ERROR_RESPONSE injection token which helps to propagate errors that occur during server-side rendering. This covers Angular's loophole in dealing with errors that occur during SSR - by default, Angular returns HTML with a status of 200, regardless of the errors that occur, which can malformed the output.
Thanks to this propagator, we can catch errors in the wrapper for CommonEngine responsible for rendering and return them instead of the malformed rendered view.
Together with this PR, we provide defaultServerErrorResponseHandlers function for ExpressJS, which helps to deal with caught errors:

falls back to the CSR with the status 404:
falls back to the CSR with the status 500:

Customer can also provide their handlers and use them inside the server.js file instead of the ones we provided.

QA steps:

Tests in storefront app:

call HTTP error that may be caught during server-side rendering e.g use HTTP proxy OR
- modify pages property in default-cms-config.ts to generate CmsPageNotFoundServerError
- modify components property in default-cms-config.ts to generate UnknownServerError
run npm run dev:ssr
open the application in the browser
verify whether the app fell back to CSR with the proper status code:
index.html returned with status 404 if pages endpoint has been modified in default-cms-config.ts
index.html returned with status 500 if components endpoint has been modified in default-cms-config.ts

Tests in customer's app:

build and deploy Spartacus packages locally:
npx ts-node ./tools/schematics/testing.ts in the SPA root folder
generate fresh ng17 application:
npx @angular/cli new my-app --standalone=false --ssr=false --style=scss --routing=false
go to the project's directory and install Spartacus with SSR from local packages:
npx @angular/cli add @spartacus/schematics@latest --ssr
use HTTP proxy or modify config for CMS endpoints to cause an error when rendering:

provideConfig(<OccConfig>{
      backend: {
        occ: {
          endpoints: {
            // components: 'cms/components2', FOR STATUS 500
            pages: 'cms/pages2', // FOR STATUS 404
          },
        },
      },
    }),

in server.ts inside app function use handlers provided by Spartacus:

  const indexHtmlContent = readFileSync(indexHtml, 'utf-8');

  /** */

  server.use(defaultServerErrorResponseHandlers(indexHtmlContent));

run npm run build and npm run serve:ssr:my-app to verify if server error responses have been handled as in test case 1

create a custom handler for errors (you can use the below example):

npm i -d @types/ejs ejs
handle-page-not-found.ts:

import { ErrorRequestHandler } from 'express';
import ejs from 'ejs';
import { CmsPageNotFoundServerErrorResponse } from '@spartacus/setup/ssr';

export const handlePageNotFound: ErrorRequestHandler = async (
  err,
  _req,
  res,
  next,
) => {
  if (err instanceof CmsPageNotFoundServerErrorResponse) {
    const html = await ejs.renderFile('./404.ejs', {
      title: 'OPS! Page not found',
      text: 'The page you are looking for does not exist',
    });
    res.status(404).send(html);
  } else {
    next(err);
  }
};

404.ejs:

<!DOCTYPE HTML>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Page Not Found</title>
  </head>
  <body>
    <h1><%= title %></h1>
    <p><%= text %></p>
  </body>
</html>

in server.ts, use handler inside app function:

 server.use(handlePageNotFound)

run npm run build and npm run serve:ssr:my-app to verify if the custom handler works

closes CXSPA-6578

…t ExpressJS handlers

Platonn · 2024-04-24T16:54:30Z

projects/storefrontapp/server.ts

+  server.use(handleCmsPageNotFoundErrorResponse(indexHtmlContent));
+  server.use(handleUnknownServerErrorResponse(indexHtmlContent));
+


Strategic question: how do we plan to distribute our error handlers to customers?

will we be adding them to their server.ts with schematics?

if yes, do we plan to put them one by one or somehow wrap them in one uber-error-handler, a.k.a. server.use(defaultServerErrorResponseHandlers()) // import defaultServerErrorResponseHandlers from @spartacus/setup/ssr?

thanks to this, over time we will be able to introduce changes/additions if needed - in existing apps. For example, we can easily add a temporaryRedirectErrorHandler (sending 302) if we really want to

Yes, the idea is to provide it to fresh SPA apps via schematics. uber-error-handler is a tempting idea giving us possibilities for easier adjustments. The reason why I decided to introduce separate handlers was the question what if the customer wants to have our handler for unknown server errors, but they custom handler with an error page for 404?
Actually, now I have an answer... the custom handler can be provided before ours. What do you think about that?

IMHO the most frequent use cases should be handled with a simple config, or super simple API. Ideally enabled implicitly by default. Medium-frequent use cases should require a tiny bit of coding, but proportionally to how often it is a use case. And for edge cases it's acceptable that customers might need to give up many OOTB default APIs and possibly write a lot of copy-pasted code + some customizations. (https://blog.platis.dev/open-source)

And in this case, in my opinion, default error handlers are good enough for most people. So, it should be a super simple and ergonomic API to use. Ideally implicit, but unfortunately, it's not possible because you have to put it explictly into the server.ts file.

If someone wants to add their error handlers - well, you nicely noticed that they can add their own above ours!! Nice catch!

If someone would like to remove one of our handlers at all, then they might want to remove our pack of error handlers. But for ergonomics, let's expose in our public APi all individuall handlers - so customers can pick and mix whatever they want, explicitly.

core-libs/setup/ssr/engine/cx-common-engine-wrapper.ts

core-libs/setup/ssr/error-handling/server-error-response/propagate-server-error-response.ts

core-libs/setup/ssr/engine/cx-common-engine-wrapper.ts

core-libs/setup/ssr/error-handling/multi-error-handlers/server-responding-error-handler.ts

Platonn · 2024-04-24T20:39:09Z

...dling/server-error-response-factory/cms-page-not-found-server-error-response-factory.spec.ts

@@ -38,23 +38,34 @@ describe('CmsPageNotFoundServerErrorResponse', () => {
    expect(cmsPageNotFoundServerErrorResponse.getPriority()).toBe(Priority.LOW);
  });

-  it('should match if error is an instance of HttpErrorResponse and URL starts with proper OCC endpoint including /cms/pages', () => {
+  it('should match if error is an instance of HttpErrorResponse and, status is 404 and URL starts with proper OCC endpoint including /cms/pages', () => {


Suggested change

it('should match if error is an instance of HttpErrorResponse and, status is 404 and URL starts with proper OCC endpoint including /cms/pages', () => {

it('should match if: error is an instance of HttpErrorResponse and status is 404 and URL starts with proper OCC endpoint including /cms/pages', () => {

Not sure whether I like the proposal more 🤔

...bs/setup/ssr/error-handling/server-error-response-handlers/server-error-response-handlers.ts

core-libs/setup/ssr/error-handling/server-error-response/propagate-server-error-response.ts

core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts

core-libs/setup/ssr/engine/cx-common-engine.spec.ts

core-libs/setup/ssr/engine/cx-common-engine.ts

...bs/setup/ssr/error-handling/server-error-response-handlers/server-error-response-handlers.ts

core-libs/setup/ssr/error-handling/server-error-response/propagate-server-error-response.ts

Co-authored-by: Krzysztof Platis <platonn.git@gmail.com>

…gate-server-error-response.ts Co-authored-by: Krzysztof Platis <platonn.git@gmail.com>

Co-authored-by: Krzysztof Platis <platonn.git@gmail.com>

core-libs/setup/ssr/engine/cx-common-engine.spec.ts

...bs/setup/ssr/error-handling/server-error-response-handlers/server-error-response-handlers.ts

feat: CXSPA-6578 Introduce PROPAGATE_SERVER_ERROR_RESPONSE and defaul…

a1da1f2

…t ExpressJS handlers

pawelfras requested a review from a team as a code owner April 22, 2024 10:55

github-actions bot and others added 3 commits April 22, 2024 10:56

Add license header

6443912

fix: sonar issues

f1e5475

Merge branch 'epic/ssr-error-handling' into feat/CXSPA-6578

4f484c3