Skip to content

Commit

Permalink
fix: wrong ICM is used when configured ICM throws errors on SSR (#1519)
Browse files Browse the repository at this point in the history
* icmBaseURL no longer uses environment.ts fallback in production mode
* make usage of default/fallback configuration in environment.ts configurable for getting state properties
* errors should only contain properties of defined HttpError interface
* do not log an error that can't be stringified, it floods the log with irrelevant information

BREAKING CHANGES: No longer falls back to `environment.ts` configured `icmBaseURL` in production mode. The given option `disableDefault` can be removed in `configuration.effects.ts` in order to retain the old behavior.
  • Loading branch information
Eisie96 committed Oct 16, 2023
1 parent 51b52ad commit 8c2c203
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 6 deletions.
3 changes: 2 additions & 1 deletion src/app/app.server.module.ts
Expand Up @@ -27,7 +27,8 @@ export class UniversalErrorHandler implements ErrorHandler {
try {
console.error('ERROR', JSON.stringify(error));
} catch (_) {
console.error('ERROR (cannot stringify)', error);
// do not log the error if it can't be stringified, it floods the log with irrelevant information
console.error('ERROR (cannot stringify)');
}
} else {
console.error('ERROR', error);
Expand Down
Expand Up @@ -65,7 +65,9 @@ export class ConfigurationEffects {
ofType(ROOT_EFFECTS_INIT),
take(1),
withLatestFrom(
this.stateProperties.getStateOrEnvOrDefault<string>('ICM_BASE_URL', 'icmBaseURL'),
this.stateProperties.getStateOrEnvOrDefault<string>('ICM_BASE_URL', 'icmBaseURL', {
disableDefault: PRODUCTION_MODE,
}),
this.stateProperties.getStateOrEnvOrDefault<string>('ICM_SERVER', 'icmServer'),
this.stateProperties.getStateOrEnvOrDefault<string>('ICM_SERVER_STATIC', 'icmServerStatic'),
this.stateProperties.getStateOrEnvOrDefault<string>('ICM_CHANNEL', 'icmChannel'),
Expand Down
6 changes: 5 additions & 1 deletion src/app/core/store/core/error/error.reducer.ts
@@ -1,4 +1,5 @@
import { createReducer, on } from '@ngrx/store';
import { pick } from 'lodash-es';

import { HttpError } from 'ish-core/models/http-error/http-error.model';

Expand All @@ -23,7 +24,10 @@ export const errorReducer = createReducer(
serverConfigError,
(state, action): ErrorState => ({
...state,
current: action.payload.error,
current:
typeof action.payload.error === 'object'
? pick(action.payload.error, 'code', 'errors', 'message', 'name', 'status')
: action.payload.error,
type: action.type,
})
)
Expand Down
14 changes: 11 additions & 3 deletions src/app/core/utils/state-transfer/state-properties.service.ts
Expand Up @@ -30,9 +30,15 @@ export class StatePropertiesService {
constructor(private store: Store) {}

/**
* Retrieve property from first set property of server state, system environment or environment.ts
* Retrieve property from first set property of server state, system environment or environment.ts (default)
* optional the fallback to default can be disabled
* e.g. for production environments where there should not be a fallback for the system environment configuration
*/
getStateOrEnvOrDefault<T>(envKey: string, envPropKey: keyof Environment): Observable<T> {
getStateOrEnvOrDefault<T>(
envKey: string,
envPropKey: keyof Environment,
options?: { disableDefault: boolean }
): Observable<T> {
return this.store.pipe(
select(getConfigurationState),
mapToProperty(envPropKey as keyof ConfigurationType),
Expand All @@ -41,8 +47,10 @@ export class StatePropertiesService {
return value;
} else if (SSR && process.env[envKey]) {
return process.env[envKey];
} else {
} else if (!options?.disableDefault) {
return environment[envPropKey];
} else {
return;
}
}),
SSR
Expand Down

0 comments on commit 8c2c203

Please sign in to comment.