Skip to content

fix(platform-server): forward BEFORE_APP_SERIALIZED errors to ErrorHandler #68173

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.

Sign up for GitHub

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

Draft
arturovt wants to merge 1 commit into
base: main
Choose a base branch
from
+56 −5
Draft

fix(platform-server): forward BEFORE_APP_SERIALIZED errors to ErrorHandler #68173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/platform-server/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
StaticProvider,
Type,
ɵannotateForHydration as annotateForHydration,
ɵINTERNAL_APPLICATION_ERROR_HANDLER as INTERNAL_APPLICATION_ERROR_HANDLER,
ɵIS_HYDRATION_DOM_REUSE_ENABLED as IS_HYDRATION_DOM_REUSE_ENABLED,
ɵSSR_CONTENT_INTEGRITY_MARKER as SSR_CONTENT_INTEGRITY_MARKER,
ɵstartMeasuring as startMeasuring,
Expand Down Expand Up @@ -194,6 +195,7 @@ export async function renderInternal(

// Run any BEFORE_APP_SERIALIZED callbacks just before rendering to string.
const environmentInjector = applicationRef.injector;
const errorHandler = environmentInjector.get(INTERNAL_APPLICATION_ERROR_HANDLER);
const callbacks = environmentInjector.get(BEFORE_APP_SERIALIZED, null);
if (callbacks) {
const asyncCallbacks: Promise<void>[] = [];
Expand All @@ -204,15 +206,16 @@ export async function renderInternal(
asyncCallbacks.push(callbackResult);
}
} catch (e) {
// Ignore exceptions.
console.warn('Ignoring BEFORE_APP_SERIALIZED Exception: ', e);
// Delegate to the application's ErrorHandler so custom handlers
// (e.g. Sentry) are notified, rather than writing directly to console.
errorHandler(e);
}
}

if (asyncCallbacks.length) {
for (const result of await Promise.allSettled(asyncCallbacks)) {
if (result.status === 'rejected') {
console.warn('Ignoring BEFORE_APP_SERIALIZED Exception: ', result.reason);
errorHandler(result.reason);
}
}
}
Expand Down
52 changes: 50 additions & 2 deletions packages/platform-server/test/integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,52 @@ class HiddenModule {}
},
);

it(
`using ${isStandalone ? 'renderApplication' : 'renderModule'} ` +
`should report to ErrorHandler when TransferState contains an unserializable value (zoneless:${zoneless})`,
async () => {
// A circular reference causes JSON.stringify (called inside toJson())
// to throw. Previously this was silently swallowed, causing the server
// to return a 200 OK without the <script id="ng-state"> tag.
function createCircularTransferStateApp(s: boolean) {
@Component({standalone: s, selector: 'app', template: ''})
class CircularApp {
constructor() {
const circular: Record<string, unknown> = {};
circular['self'] = circular;
coreInject(TransferState).set(makeStateKey<unknown>('key'), circular);
}
}
return CircularApp;
}

const consoleSpy = spyOn(console, 'error');
const options = {document: doc};
const bootstrap = isStandalone
? renderApplication(
getStandaloneBootstrapFn(createCircularTransferStateApp(true)),
options,
)
: renderModule(
(() => {
const CircularApp = createCircularTransferStateApp(false);
@NgModule({
declarations: [CircularApp],
imports: [BrowserModule, ServerModule],
bootstrap: [CircularApp],
})
class M {}
return M;
})(),
options,
);
await bootstrap;
// The circular reference error is forwarded to ErrorHandler rather
// than thrown, so the render completes but the error is still reported.
expect(consoleSpy).toHaveBeenCalled();
},
);

it(
'uses `other` as the `serverContext` value when all symbols are removed after sanitization' +
`(standalone:${isStandalone}, zoneless:${zoneless})`,
Expand Down Expand Up @@ -1102,7 +1148,7 @@ class HiddenModule {}
'should call multiple render hooks' +
`(standalone:${isStandalone}, zoneless:${zoneless})`,
async () => {
const consoleSpy = spyOn(console, 'warn');
const consoleSpy = spyOn(console, 'error');
const options = {document: doc};
const bootstrap = isStandalone
? renderApplication(
Expand All @@ -1116,6 +1162,8 @@ class HiddenModule {}
'<html><head><title>RenderHook</title><meta name="description"></head>' +
'<body><app ng-version="0.0.0-PLACEHOLDER" ng-server-context="other">Works!</app></body></html>',
);
// Errors from callbacks are forwarded to ErrorHandler (console.error by default)
// rather than swallowed silently or thrown.
expect(consoleSpy).toHaveBeenCalled();
},
);
Expand Down Expand Up @@ -1143,7 +1191,7 @@ class HiddenModule {}
'should call multiple async and sync render hooks' +
`(standalone:${isStandalone}, zoneless:${zoneless})`,
async () => {
const consoleSpy = spyOn(console, 'warn');
const consoleSpy = spyOn(console, 'error');
const options = {document: doc};
const bootstrap = isStandalone
? renderApplication(
Expand Down
Loading