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-6890 Create toggle and optimization options for propagating errors to the server #19021

Merged
merged 12 commits into from
Jul 15, 2024
Original file line number Diff line number Diff line change
@@ -1,39 +1,56 @@
import { TestBed } from '@angular/core/testing';
import { FeatureConfigService } from '@spartacus/core';
import { PROPAGATE_ERROR_TO_SERVER } from '../error-response/propagate-error-to-server';
import { PropagatingToServerErrorHandler } from './propagating-to-server-error-handler';

describe('PropagatingToServerErrorHandler', () => {
describe('default factories', () => {
let serverRespondingErrorHandler: PropagatingToServerErrorHandler;
let propagatingToServerErrorHandler: PropagatingToServerErrorHandler;
let featureConfigService: FeatureConfigService;
let propagateErrorResponse: any;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [
PropagatingToServerErrorHandler,
FeatureConfigService,
{
provide: PROPAGATE_ERROR_TO_SERVER,
useValue: jest.fn(),
},
],
});

serverRespondingErrorHandler = TestBed.inject(
propagatingToServerErrorHandler = TestBed.inject(
PropagatingToServerErrorHandler
);
propagateErrorResponse = TestBed.inject(PROPAGATE_ERROR_TO_SERVER);
featureConfigService = TestBed.inject(FeatureConfigService);
});

afterEach(() => {
jest.clearAllMocks();
});

it('should propagate error', () => {
it('should propagate error when propagateErrorsToServer is enabled', () => {
jest
.spyOn(featureConfigService, 'isEnabled')
.mockImplementationOnce((val) => val === 'propagateErrorsToServer');
const error = new Error('test error');

serverRespondingErrorHandler.handleError(error);
propagatingToServerErrorHandler.handleError(error);

expect(propagateErrorResponse as jest.Mock).toHaveBeenCalledWith(error);
});

it('should not propagate error when propagateErrorsToServer is disabled', () => {
jest
.spyOn(featureConfigService, 'isEnabled')
.mockImplementationOnce((val) => !(val === 'propagateErrorsToServer'));
const error = new Error('test error');

propagatingToServerErrorHandler.handleError(error);

expect(propagateErrorResponse as jest.Mock).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { Injectable, inject } from '@angular/core';
import { MultiErrorHandler } from '@spartacus/core';
import { FeatureConfigService, MultiErrorHandler } from '@spartacus/core';
import { PROPAGATE_ERROR_TO_SERVER } from '../error-response/propagate-error-to-server';

/**
Expand All @@ -28,8 +28,13 @@ import { PROPAGATE_ERROR_TO_SERVER } from '../error-response/propagate-error-to-
})
export class PropagatingToServerErrorHandler implements MultiErrorHandler {
protected propagateErrorToServer = inject(PROPAGATE_ERROR_TO_SERVER);
private featureConfigService: FeatureConfigService =
inject(FeatureConfigService);

handleError(error: unknown): void {
if (!this.featureConfigService.isEnabled('propagateErrorsToServer')) {
return;
}
this.propagateErrorToServer(error);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`OptimizedSsrEngine avoidCachingErrors option when using default cacheStrategyResolver should cache errors if \`avoidCachingErrors\` is set to false 1`] = `
[
[Error: a-0],
[Error: a-0],
[Error: a-0],
]
`;

exports[`OptimizedSsrEngine avoidCachingErrors option when using default cacheStrategyResolver should not cache errors if \`avoidCachingErrors\` is set to true 1`] = `
[
[Error: a-0],
[Error: a-1],
[Error: a-2],
]
`;

exports[`OptimizedSsrEngine cacheStrategyResolver option should cache errors if \`cacheStrategyResolver\` returns true 1`] = `
[
[Error: a-0],
[Error: a-0],
[Error: a-0],
]
`;

exports[`OptimizedSsrEngine cacheStrategyResolver option should not cache errors if \`cacheStrategyResolver\` returns false 1`] = `
[
[Error: a-0],
[Error: a-1],
[Error: a-2],
]
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`RenderingCache with cacheSize RenderingCache and cacheStrategyResolver if default cacheStrategyResolver should cache if avoidCachingErrors is false 1`] = `
{
"err": [Error: err],
"html": "a",
}
`;
1 change: 1 addition & 0 deletions core-libs/setup/ssr/optimized-engine/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

export * from './optimized-ssr-engine';
export * from './rendering-cache';
export * from './rendering-cache.model';
export * from './rendering-strategy-resolver';
export * from './rendering-strategy-resolver-options';
export { RequestContext, getRequestContext } from './request-context';
Expand Down
184 changes: 149 additions & 35 deletions core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jest.mock('fs', () => ({
readFileSync: () => '',
}));
const consoleLogSpy = jest.spyOn(console, 'log').mockImplementation();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();

class MockExpressServerLogger implements Partial<ExpressServerLogger> {
log(message: string, context: ExpressServerLoggerContext): void {
Expand All @@ -39,7 +40,7 @@ class MockExpressServerLogger implements Partial<ExpressServerLogger> {
*/
class TestEngineRunner {
/** Accumulates html output for engine runs */
renders: string[] = [];
renders: (string | Error)[] = [];
Platonn marked this conversation as resolved.
Show resolved Hide resolved

/** Accumulates response parameters for engine runs */
responseParams: object[] = [];
Expand All @@ -48,25 +49,45 @@ class TestEngineRunner {
optimizedSsrEngine: OptimizedSsrEngine;
engineInstance: NgExpressEngineInstance;

constructor(options: SsrOptimizationOptions, renderTime?: number) {
constructor(
options: SsrOptimizationOptions,
renderTime?: number,
params?: { withError?: boolean }
) {
// mocked engine instance that will render test output in 100 milliseconds
const engineInstanceMock = (
filePath: string,
_: any,
callback: SsrCallbackFn
) => {
setTimeout(() => {
callback(undefined, `${filePath}-${this.renderCount++}`);
const result = `${filePath}-${this.renderCount++}`;

if (params?.withError) {
const err = new Error(result);
callback(err, undefined);
} else {
callback(undefined, result);
}
}, renderTime ?? defaultRenderTime);
};

this.optimizedSsrEngine = new OptimizedSsrEngine(
engineInstanceMock,
options
);
this.optimizedSsrEngine = new OptimizedSsrEngine(engineInstanceMock, {
cacheStrategyResolver:
defaultSsrOptimizationOptions.cacheStrategyResolver,
...options,
});
this.engineInstance = this.optimizedSsrEngine.engineInstance;
}

/** Create engine that results with error during render */
static withError(
options: SsrOptimizationOptions,
renderTime = defaultRenderTime
): TestEngineRunner {
return new TestEngineRunner(options, renderTime, { withError: true });
}

/** Run request against the engine. The result will be stored in rendering property. */
request(
url: string,
Expand Down Expand Up @@ -102,8 +123,8 @@ class TestEngineRunner {
},
};

this.engineInstance(url, optionsMock, (_, html): void => {
this.renders.push(html ?? '');
this.engineInstance(url, optionsMock, (error, html): void => {
this.renders.push(html ?? error ?? '');
this.responseParams.push(response);
});

Expand Down Expand Up @@ -177,7 +198,9 @@ describe('OptimizedSsrEngine', () => {
"reuseCurrentRendering": true,
"debug": false,
"renderingStrategyResolver": "() => ssr_optimization_options_1.RenderingStrategy.ALWAYS_SSR",
"logger": "DefaultExpressServerLogger"
"logger": "DefaultExpressServerLogger",
"cacheStrategyResolver": "(options, entry) => !(options.avoidCachingErrors === true && Boolean(entry.err))",
"avoidCachingErrors": false
}
}
}",
Expand All @@ -186,6 +209,34 @@ describe('OptimizedSsrEngine', () => {
});
});

describe('rendering', () => {
it('should return rendered output if no errors', fakeAsync(() => {
const originalUrl = 'a';
const engineRunner = new TestEngineRunner({}).request('a');

tick(200);
expect(engineRunner.renders).toEqual(['a-0']);
expect(consoleLogSpy).toHaveBeenCalledWith(
expect.stringContaining(
`Request is resolved with the SSR rendering result (${originalUrl})`
)
);
}));

it('should return error if rendering fails', fakeAsync(() => {
const originalUrl = 'a';
const engineRunner = TestEngineRunner.withError({}).request('a');

tick(200);
expect(engineRunner.renders).toEqual([new Error('a-0')]);
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining(
`Request is resolved with the SSR rendering error (${originalUrl})`
)
);
}));
});

describe('rendering cache', () => {
it('should be initialized with default optimization options none of the custom options are provided', () => {
const engineRunner = new TestEngineRunner({});
Expand Down Expand Up @@ -305,7 +356,6 @@ describe('OptimizedSsrEngine', () => {
tick(200);
expect(engineRunner.renders).toEqual(['a-0', 'a-1', 'a-2']);
}));

it('should cache requests if enabled', fakeAsync(() => {
const engineRunner = new TestEngineRunner({
cache: true,
Expand Down Expand Up @@ -335,6 +385,68 @@ describe('OptimizedSsrEngine', () => {
}));
});

describe('avoidCachingErrors option', () => {
describe('when using default cacheStrategyResolver', () => {
it('should not cache errors if `avoidCachingErrors` is set to true', fakeAsync(() => {
const engineRunner = TestEngineRunner.withError({
cache: true,
avoidCachingErrors: true,
}).request('a');

tick(200);
engineRunner.request('a');
tick(200);
engineRunner.request('a');
tick(200);
expect(engineRunner.renders).toMatchSnapshot();
pawelfras marked this conversation as resolved.
Show resolved Hide resolved
}));

it('should cache errors if `avoidCachingErrors` is set to false', fakeAsync(() => {
const engineRunner = TestEngineRunner.withError({
cache: true,
avoidCachingErrors: false,
}).request('a');

tick(200);
engineRunner.request('a');
tick(200);
engineRunner.request('a');
tick(200);
expect(engineRunner.renders).toMatchSnapshot();
}));
});
});

describe('cacheStrategyResolver option', () => {
it('should not cache errors if `cacheStrategyResolver` returns false', fakeAsync(() => {
const engineRunner = TestEngineRunner.withError({
cache: true,
cacheStrategyResolver: () => false,
}).request('a');

tick(200);
engineRunner.request('a');
tick(200);
engineRunner.request('a');
tick(200);
expect(engineRunner.renders).toMatchSnapshot();
}));

it('should cache errors if `cacheStrategyResolver` returns true', fakeAsync(() => {
const engineRunner = TestEngineRunner.withError({
cache: true,
cacheStrategyResolver: () => true,
}).request('a');

tick(200);
engineRunner.request('a');
tick(200);
engineRunner.request('a');
tick(200);
expect(engineRunner.renders).toMatchSnapshot();
}));
});
pawelfras marked this conversation as resolved.
Show resolved Hide resolved

describe('concurrency option', () => {
it('should limit concurrency and fallback to csr', fakeAsync(() => {
const engineRunner = new TestEngineRunner({
Expand Down Expand Up @@ -1269,30 +1381,32 @@ describe('OptimizedSsrEngine', () => {
logger: new MockExpressServerLogger() as ExpressServerLogger,
});
expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(`
[
"[spartacus] SSR optimization engine initialized",
{
"options": {
"cacheSize": 3000,
"concurrency": 10,
"debug": false,
"forcedSsrTimeout": 60000,
"logger": "MockExpressServerLogger",
"maxRenderTime": 300000,
"renderingStrategyResolver": "(request) => {
if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) {
return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR;
}
return shouldFallbackToCsr(request, options)
? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR
: ssr_optimization_options_1.RenderingStrategy.DEFAULT;
}",
"reuseCurrentRendering": true,
"timeout": 3000,
},
},
]
`);
[
"[spartacus] SSR optimization engine initialized",
{
"options": {
"avoidCachingErrors": false,
"cacheSize": 3000,
"cacheStrategyResolver": "(options, entry) => !(options.avoidCachingErrors === true && Boolean(entry.err))",
"concurrency": 10,
"debug": false,
"forcedSsrTimeout": 60000,
"logger": "MockExpressServerLogger",
"maxRenderTime": 300000,
"renderingStrategyResolver": "(request) => {
if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) {
return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR;
}
return shouldFallbackToCsr(request, options)
? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR
: ssr_optimization_options_1.RenderingStrategy.DEFAULT;
}",
"reuseCurrentRendering": true,
"timeout": 3000,
},
},
]
`);
});
});
});
Loading
Loading