[DO NOT MERGE] CXSPA-6939 (PoC) Handling breaking changes related to SSR Error Handling

core-libs/setup/ssr/error-handling/enable-ssr-error-handling.ts

@@ -0,0 +1,14 @@
+/*
+ * SPDX-FileCopyrightText: 2024 SAP Spartacus team <spartacus-team@sap.com>
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { InjectionToken } from '@angular/core';
+
+/**
+ * Toggle for enabling the SSR error handling.
+ */
+export const ENABLE_SSR_ERROR_HANDLING = new InjectionToken<boolean>(
+  'ENABLE_SSR_ERROR_HANDLING'
+);

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

@@ -6,6 +6,7 @@
 
 import { Injectable, inject } from '@angular/core';
 import { MultiErrorHandler, resolveApplicable } from '@spartacus/core';
+import { ENABLE_SSR_ERROR_HANDLING } from '../enable-ssr-error-handling';
 import { SERVER_ERROR_RESPONSE_FACTORY } from '../server-error-response-factory';
 import { PROPAGATE_SERVER_ERROR_RESPONSE } from '../server-error-response/propagate-server-error-response';
 
@@ -25,15 +26,18 @@ export class ServerRespondingErrorHandler implements MultiErrorHandler {
   protected propagateServerErrorResponse = inject(
     PROPAGATE_SERVER_ERROR_RESPONSE
   );
+  protected isSsrErrorHandlingEnabled = inject(ENABLE_SSR_ERROR_HANDLING);
 
   handleError(error: unknown): void {
-    const cxServerErrorResponse = resolveApplicable(
-      this.serverErrorResponseFactories,
-      [error]
-    )?.create(error);
+    if (this.isSsrErrorHandlingEnabled) {
+      const cxServerErrorResponse = resolveApplicable(
+        this.serverErrorResponseFactories,
+        [error]
+      )?.create(error);
 
-    if (cxServerErrorResponse) {
-      this.propagateServerErrorResponse(cxServerErrorResponse);
+      if (cxServerErrorResponse) {
+        this.propagateServerErrorResponse(cxServerErrorResponse);
+      }
     }
   }
 }

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

@@ -8,6 +8,7 @@
 import { Request, Response } from 'express';
 import * as fs from 'fs';
 import { NgExpressEngineInstance } from '../engine-decorator/ng-express-engine-decorator';
+import { ENABLE_SSR_ERROR_HANDLING } from '../error-handling/enable-ssr-error-handling';
 import { getRequestUrl } from '../express-utils/express-request-url';
 import {
   EXPRESS_SERVER_LOGGER,
@@ -445,6 +446,19 @@ export class OptimizedSsrEngine {
           provide: EXPRESS_SERVER_LOGGER,
           useValue: this.logger,
         },
+        /*
+        For SSR error handling purposes, we could introduce new token responsible for toggling the error handling and use it across the application.
+        This means we introduce another standard for for toggling features:
+        Pros:
+        + it won't bother customers using CRS
+
+        Cons:
+        - inconsistency in the way of toggling features
+        */
+        {
+          provide: ENABLE_SSR_ERROR_HANDLING,
+          useValue: this.ssrOptions?.ssrErrorHandling,
+        },
         ...(options?.providers ?? []),
       ],
     };

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

@@ -37,7 +37,15 @@ export class RenderingCache {
         this.renders.delete(this.renders.keys().next().value);
       }
     }
-    this.renders.set(key, entry);
+    // cache only if cachingStrategyResolver return true
+    // Fresh apps: use new default caching strategy == do not cache errors
+    // Legacy apps: do not provide new default caching strategy to keep the old behavior
+    if (
+      this.options?.cachingStrategyResolver &&
+      !this.options?.cachingStrategyResolver(entry)
+    ) {
+      this.renders.set(key, entry);
+    }
   }
 
   get(key: string): RenderingEntry | undefined {

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

@@ -130,6 +130,20 @@ export interface SsrOptimizationOptions {
    * By default, the DefaultExpressServerLogger is used.
    */
   logger?: ExpressServerLogger;
+
+  /**
+   * WIP: Custom caching strategy resolver.
+   * By default, the caching strategy is based on the presence of an error.
+   */
+  cachingStrategyResolver?: (entry: {
+    error?: Error | unknown;
+    html?: string;
+  }) => boolean;
+
+  /**
+   * Toggle for enabling the SSR error handling.
+   */
+  ssrErrorHandling?: boolean;
 }
 
 export enum RenderingStrategy {
@@ -150,4 +164,6 @@ export const defaultSsrOptimizationOptions: SsrOptimizationOptions = {
     defaultRenderingStrategyResolverOptions
   ),
   logger: new DefaultExpressServerLogger(),
+  cachingStrategyResolver: (entry) => !entry.error,
+  ssrErrorHandling: true,
 };

core-libs/setup/ssr/providers/ssr-providers.ts

@@ -4,14 +4,16 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { Provider, StaticProvider } from '@angular/core';
+import { Provider, StaticProvider, inject } from '@angular/core';
 import {
   LoggerService,
   MULTI_ERROR_HANDLER,
   SERVER_REQUEST_ORIGIN,
   SERVER_REQUEST_URL,
+  provideFeatureTogglesFactory,
 } from '@spartacus/core';
 
+import { ENABLE_SSR_ERROR_HANDLING } from '../error-handling/enable-ssr-error-handling';
 import { ServerRespondingErrorHandler } from '../error-handling/multi-error-handlers';
 import { provideServerErrorResponseFactories } from '../error-handling/server-error-response-factory/provide-server-error-response-factories';
 import { getRequestOrigin } from '../express-utils/express-request-origin';
@@ -40,11 +42,19 @@ export function provideServer(options?: ServerOptions): Provider[] {
       useFactory: serverLoggerServiceFactory,
     },
     {
-      provide: MULTI_ERROR_HANDLER,
+      provide: MULTI_ERROR_HANDLER, // we're going to provide it, but we'll enable this feature in other place
       useExisting: ServerRespondingErrorHandler,
       multi: true,
     },
-    provideServerErrorResponseFactories(),
+    provideServerErrorResponseFactories(), // we're going to provide it, but we'll enable this feature in other place
+    // TBD: to ensure that the necessary feature toggles are available if the feature is enabled
+    provideFeatureTogglesFactory(() => {
+      const isSsrErrorHandlingEnabled = inject(ENABLE_SSR_ERROR_HANDLING);
+      return {
+        ngrxErrorHandling: isSsrErrorHandlingEnabled,
+        httpErrorHandling: isSsrErrorHandlingEnabled,
+      };
+    }),
   ];
 }
 /**

feature-libs/asm/core/store/actions/customer.action.ts

@@ -5,7 +5,7 @@
  */
 
 import { CustomerSearchOptions, CustomerSearchPage } from '@spartacus/asm/root';
-import { StateUtils, ErrorActionType } from '@spartacus/core';
+import { ErrorActionType, StateUtils } from '@spartacus/core';
 import {
   CUSTOMER_LIST_CUSTOMERS_SEARCH_DATA,
   CUSTOMER_SEARCH_DATA,
@@ -32,9 +32,20 @@ export class CustomerSearch extends StateUtils.LoaderLoadAction {
   }
 }
 
+// possible ways of handling breaking changes in ALL actions:
+// - introduce CustomerSearchFailV2 and deprecate old class
+// - overload the constructor of CustomerSearchFail and handle the change there together with deprecating he old one
+
 export class CustomerSearchFail extends StateUtils.LoaderFailAction {
   readonly type = CUSTOMER_SEARCH_FAIL;
+  constructor(error: ErrorActionType);
+  /**
+   * @deprecated please use constructor(error: ErrorActionType) instead
+   */
+  //eslint-disable-next-line @typescript-eslint/unified-signatures
+  constructor(error: any);
   constructor(public error: ErrorActionType) {
+    //do we consider changes in NgRx as breaking? if so, we  should handle it (somehow)
     super(CUSTOMER_SEARCH_DATA, error);
   }
 }
@@ -62,6 +73,12 @@ export class CustomerListCustomersSearch extends StateUtils.LoaderLoadAction {
 
 export class CustomerListCustomersSearchFail extends StateUtils.LoaderFailAction {
   readonly type = CUSTOMER_LIST_CUSTOMERS_SEARCH_FAIL;
+  constructor(payload: ErrorActionType);
+  /**
+   * @deprecated please use constructor(payload: ErrorActionType) instead
+   */
+  //eslint-disable-next-line @typescript-eslint/unified-signatures
+  constructor(payload: any);
   constructor(public payload: ErrorActionType) {
     super(CUSTOMER_LIST_CUSTOMERS_SEARCH_DATA, payload);
   }

projects/core/src/base-core.module.ts

@@ -10,7 +10,6 @@ import { ConfigInitializerModule } from './config/config-initializer/config-init
 import { ConfigValidatorModule } from './config/config-validator/config-validator.module';
 import { ConfigModule } from './config/config.module';
 import { ErrorHandlingModule, HttpErrorHandlerModule } from './error-handling';
-import { EffectsErrorHandlerModule } from './error-handling/effects-error-handler/effects-error-handler.module';
 import { FeaturesConfigModule } from './features-config/features-config.module';
 import { GlobalMessageModule } from './global-message/global-message.module';
 import { HttpModule } from './http/http.module';
@@ -24,6 +23,7 @@ import { StateModule } from './state/state.module';
 
 @NgModule({
   imports: [
+    // breaking change - HttpErrorHandlerInterceptor may have featureService inside manipulate the behavior (implemented) OR we can inform in docs that this module has to be imported
     HttpErrorHandlerModule.forRoot(), // Import this module before any other interceptor to handle HTTP errors efficiently
     StateModule.forRoot(),
     ConfigModule.forRoot(),
@@ -40,7 +40,6 @@ import { StateModule } from './state/state.module';
     LazyLoadingModule.forRoot(),
     HttpModule.forRoot(),
     ErrorHandlingModule.forRoot(),
-    EffectsErrorHandlerModule.forRoot(),
   ],
 })
 export class BaseCoreModule {

projects/core/src/error-handling/effects-error-handler/cx-error-handler.effect.ts

@@ -5,28 +5,30 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { Injectable } from '@angular/core';
+import { Injectable, inject } from '@angular/core';
 import { Actions, createEffect } from '@ngrx/effects';
+import { Observable } from 'rxjs';
 import { filter, tap } from 'rxjs/operators';
-import { EffectsErrorHandlerService } from './effects-error-handler.service';
+import { FeatureConfigService } from '../../features-config';
 import { ErrorAction } from '../../model/index';
-import { Observable } from 'rxjs';
+import { EffectsErrorHandlerService } from './effects-error-handler.service';
 
 @Injectable()
 export class CxErrorHandlerEffect {
+  protected actions$ = inject(Actions);
+  protected effectErrorHandler = inject(EffectsErrorHandlerService);
+  protected featureConfigService = inject(FeatureConfigService);
+
   error$: Observable<ErrorAction> = createEffect(
     () =>
       this.actions$.pipe(
         filter(this.effectErrorHandler.filterActions),
-        tap((errorAction: ErrorAction) =>
-          this.effectErrorHandler.handleError(errorAction)
-        )
+        tap((errorAction) => {
+          if (this.featureConfigService.isEnabled('ngrxErrorHandling')) {
+            this.effectErrorHandler.handleError(errorAction);
+          }
+        })
       ),
     { dispatch: false }
   );
-
-  constructor(
-    protected actions$: Actions,
-    protected effectErrorHandler: EffectsErrorHandlerService
-  ) {}
 }

projects/core/src/error-handling/effects-error-handler/effects-error-handler.service.ts

@@ -5,18 +5,18 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { ErrorHandler, Injectable } from '@angular/core';
+import { HttpErrorResponse } from '@angular/common/http';
+import { ErrorHandler, Injectable, inject } from '@angular/core';
 import { Action } from '@ngrx/store';
 import {
   ErrorAction,
   ErrorActionType,
   HttpErrorModel,
 } from '../../model/index';
-import { HttpErrorResponse } from '@angular/common/http';
 
 @Injectable()
 export class EffectsErrorHandlerService {
-  constructor(protected errorHandler: ErrorHandler) {}
+  protected errorHandler = inject(ErrorHandler);
 
   handleError(action: ErrorAction): void {
     const error: ErrorActionType = action.error;

projects/core/src/error-handling/error-handling.module.ts

@@ -6,15 +6,21 @@
 
 import { ErrorHandler, ModuleWithProviders, NgModule } from '@angular/core';
 import { CxErrorHandler } from './cx-error-handler';
+import { EffectsErrorHandlerModule } from './effects-error-handler';
 import { provideMultiErrorHandler } from './multi-error-handler';
 
-@NgModule()
+// maybe this module should contain all error handling related parts, including modules providing breaking changes?
+// this module is already part of BaseCoreModule
+// verify whether importing HttpErrorHandler module here won't break its behavior.
+@NgModule({
+  imports: [EffectsErrorHandlerModule.forRoot()],
+})
 export class ErrorHandlingModule {
   static forRoot(): ModuleWithProviders<ErrorHandlingModule> {
     return {
       ngModule: ErrorHandlingModule,
       providers: [
-        provideMultiErrorHandler(),
+        provideMultiErrorHandler(), // potentially not breaking change (?)
         {
           provide: ErrorHandler,
           useClass: CxErrorHandler,

projects/core/src/error-handling/http-error-handler/http-error-handler.interceptor.ts

@@ -5,16 +5,17 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { ErrorHandler, Injectable } from '@angular/core';
 import {
   HttpErrorResponse,
   HttpEvent,
   HttpHandler,
   HttpInterceptor,
   HttpRequest,
 } from '@angular/common/http';
+import { ErrorHandler, Injectable, inject } from '@angular/core';
 import { Observable } from 'rxjs';
 import { tap } from 'rxjs/operators';
+import { FeatureConfigService } from '../../features-config';
 
 /**
  * This interceptor forwards all HTTP errors (e.g. 5xx or 4xx status response from backend)
@@ -25,12 +26,17 @@ import { tap } from 'rxjs/operators';
  */
 @Injectable()
 export class HttpErrorHandlerInterceptor implements HttpInterceptor {
-  constructor(protected errorHandler: ErrorHandler) {}
+  protected errorHandler = inject(ErrorHandler);
+  protected featureService = inject(FeatureConfigService);
 
   intercept(
     request: HttpRequest<any>,
     next: HttpHandler
   ): Observable<HttpEvent<any>> {
+    //double-check whether it is good way of handling HTTP errors from api calls
+    if (!this.featureService.isEnabled('httpErrorHandling')) {
+      return next.handle(request);
+    }
     return next.handle(request).pipe(
       tap({
         error: (error) => {

projects/core/src/features-config/feature-toggles/config/feature-toggles.ts

@@ -43,6 +43,17 @@ export interface FeatureTogglesInterface {
    */
   productConfiguratorAttributeTypesV2?: boolean;
 
+  /*
+  I have hard time thinking about how many feature toggles we should introduce:
+  i would say we need:
+  - separate flag for toggling HTTP error handling
+  - separate flag for NgRx error handling?
+  - separate flag for SSR error handling? OR EVENT IT SHOULD NOT BE A FEATURE TOGGLE? (see comment in OptimizedSsrEngine)
+
+  */
+  ngrxErrorHandling?: boolean;
+  httpErrorHandling?: boolean;
+
   /**
    * Adds asterisks to required form fields in all components existing before v2211.20
    */
@@ -160,6 +171,8 @@ export const defaultFeatureToggles: Required<FeatureTogglesInterface> = {
   pdfInvoicesSortByInvoiceDate: false,
   storeFrontLibCardParagraphTruncated: false,
   productConfiguratorAttributeTypesV2: false,
+  ngrxErrorHandling: false,
+  httpErrorHandling: false,
   a11yRequiredAsterisks: false,
   a11yQuantityOrderTabbing: false,
   a11yNavigationUiKeyboardControls: false,

projects/core/src/state/utils/entity-loader/entity-loader.action.ts

@@ -78,7 +78,7 @@ export class EntityLoadAction implements EntityLoaderAction {
 export class EntityFailAction implements EntityLoaderAction {
   type = ENTITY_FAIL_ACTION;
   readonly meta: EntityLoaderMeta;
-  constructor(entityType: string, id: EntityId, public error?: any) {
+  constructor(entityType: string, id: EntityId, error?: any) {
     this.meta = entityFailMeta(entityType, id, error);
   }
 }