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

feat: CXSPA-2066 SSR Error Handling #18742

Merged
merged 76 commits into from
Sep 10, 2024
Merged

Conversation

pawelfras
Copy link
Contributor

@pawelfras pawelfras commented Apr 18, 2024

This PR contains one of the missing puzzles required to enjoy SSR provided by Spartacus - Error Handling.

In the current shape of Angular SSR, that Spartacus uses under the hood, if any async error occurs during rendering, the page renders without any information on whether the error occurred and an app returns the effect of rendering, potentially malformed, due to e.g. runtime error in the app logic, or due to the HTTP error when calling API, with the wrong, misleading status of 200. As a consequence, pages that encountered errors while rendering and are possibly missing some data are indexed by Google.

The same issue occurs, when customer enters the unknown URL - Spartacus returns an error page, but with misleading status 200. Because of that, unknown URLs are indexed by Google.

Both scenarios may drastically affect the SEO of the page.

To solve the issues mentioned above, Spartacus introduces SSR Error Handling to properly react to errors that occur during the rendering page on the server and to inform the user (and Google crawlers) about the encountered issue and its type.

The whole feature is controlled by three feature flags:

  • enable the propagateErrorsToServer feature toggle to propagate to the ExpressJS the async error captured during the rendering,
  • enable ssrStrictErrorHandlingForHttpAndNgrx feature toggle to capture async HTTP errors as well as runtime errors in NgRx flow,
  • enable ssrFeatureToggle.avoidCachingErrors in SsrOptimizationOptions to not cache error pages

Architectural details:

  • introduced HttpErrorHandlerinterceptor responsible for capturing HTTP errors and forwarding wrapped errors based on their cause:
    • CmsPageNotFoundOutboundHttpError if 404 HTTP error occurred when calling cms/pages
    • OnboundHttpError if any other HTTP error occurred (i.e. non-404 in cms/pages OR any http error in any other API call)
  • introduced CxErrorHandlerEffect that captures NgRx actions of type ErrorAction and forwards error from their payload to the error handler.
  • introduced multi-provided error handlers
    • LoggingErrorHandler using LoggingService under the hood to log captured errors
    • PropagatingToServerErrorHandler that propagates error capturied during SSR to CxCommonEngine
  • introduced CxCommonEngine - wrapper for Angular's CommonEngine that handles propagated errors during SSR
  • introduced defaultEspressErrorHandlers middleware for ExpressJS that handles captured errors and falls back to CSR with the proper response status
  • provided changes to RenderingCache that help to avoid caching renders when an error occurs:
    • this can be controlled by the new optimization property ssrFeatureToggles.avoidCachingErrors (boolean, feature toggle which will be eventually removed due to breaking changes policy) or shouldCacheRenderingResult (function)
  • provided changes to OptimizedSsrEngine to inform that rendering results with error (for better traceability)

Apart from a new functionality for customers, this PR introduces:

  • new eslint rule no-ngrx-fail-action-without-error-action-implementation which makes sure that NgRx failure actions implement the ErrorAction interface necessary for an error in the NgRx flow to be caught
  • a framework for SSR E2E testing

closes CXSPA-2066

Platonn and others added 15 commits July 10, 2023 10:54
Previous behavior: When `/products` endpoint returned a http error, the code broke in [this line](https://github.com/SAP/spartacus/blob/ed1e1a78c488b1e1214491ffa736612287f8cf70/projects/core/src/product/store/effects/product.effect.ts#L77), complaining that `this` is undefined.

Fix: Preserve the context of `this` which was lost in [this line](https://github.com/SAP/spartacus/blob/ed1e1a78c488b1e1214491ffa736612287f8cf70/projects/core/src/product/store/effects/product.effect.ts#L52)

The problem was revealed only after we implemented [CXSPA-2251](https://jira.tools.sap/browse/CXSPA-2251) where we referenced `this` by adding `this.logger` to the method `ProductEffects.productLoadEffect`

fixes https://jira.tools.sap/browse/CXSPA-3902
Co-authored-by: Krzysztof Platis <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Krzysztof Platis <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
# Conflicts:
#	projects/core/src/product/store/actions/product-references.action.ts
#	projects/core/src/product/store/actions/product-reviews.action.ts
#	projects/core/src/product/store/effects/product-reviews.effect.ts
#	projects/core/src/state/utils/entity-loader/entity-loader.action.ts
This pull request introduces methodologies for integrating multiple error interceptors that manage errors within the Server-Side Rendering (SSR) framework. This architectural augmentation preserves backward compatibility, mitigating any potential disruptions for end-users upon the incorporation of new error interceptors into the system.

With the introduction of this enhancement, it becomes easier for users to include new error interceptors, giving them the flexibility to determine the order in which these interceptors are applied within the system. This priority setting allows users to control how these interceptors operate and influence the workflow of the system.

The order is:
High priority
Normal or no priority
Low priority

Preserves the original order within a group of interceptors with the same priority.
@pawelfras pawelfras requested a review from a team as a code owner April 18, 2024 14:07
@pawelfras pawelfras requested a review from a team April 18, 2024 14:07
@pawelfras pawelfras requested review from a team as code owners April 18, 2024 14:07
@github-actions github-actions bot marked this pull request as draft April 18, 2024 14:07
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft September 10, 2024 12:54
@pawelfras pawelfras marked this pull request as ready for review September 10, 2024 12:54
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft September 10, 2024 13:52
@pawelfras pawelfras marked this pull request as ready for review September 10, 2024 13:59
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft September 10, 2024 14:10
@pawelfras pawelfras marked this pull request as ready for review September 10, 2024 14:18
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft September 10, 2024 14:26
@pawelfras pawelfras marked this pull request as ready for review September 10, 2024 14:29
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build. 
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft September 10, 2024 15:37
@FollowTheFlo FollowTheFlo marked this pull request as ready for review September 10, 2024 17:21
@FollowTheFlo FollowTheFlo merged commit e2d50b7 into develop Sep 10, 2024
32 checks passed
@FollowTheFlo FollowTheFlo deleted the epic/ssr-error-handling branch September 10, 2024 18:41
Pio-Bar pushed a commit that referenced this pull request Sep 11, 2024
@Platonn Platonn restored the epic/ssr-error-handling branch September 18, 2024 10:54
@Platonn Platonn deleted the epic/ssr-error-handling branch September 18, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants