Skip to content

Gracefully handle when some config.app.* properties are missing (@W-19453183@) #3230

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

Merged
vmarta merged 20 commits into from
Sep 3, 2025
Merged

Gracefully handle when some config.app.* properties are missing (@W-19453183@) #3230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c538dac
Don't assume that config.app.* properties exist
vmarta Aug 28, 2025
8cb30b7
Don't assume that config.app.* properties exist
vmarta Aug 28, 2025
ca5f1bb
Add todos
vmarta Aug 28, 2025
287a823
Use the default config for Commerce Agent when necessary
vmarta Aug 28, 2025
6c79acb
Debugging
vmarta Aug 28, 2025
0bd299f
Merge remote-tracking branch 'origin/develop' into vm/fix-config-parsing
vmarta Aug 29, 2025
2022342
Move utils file to somewhere more upgradable and extensible
vmarta Aug 29, 2025
9e4bc9b
No longer need these .hbs files
vmarta Aug 29, 2025
159fd5a
Merge remote-tracking branch 'origin/develop' into vm/fix-config-parsing
vmarta Aug 29, 2025
78d9185
Sync the corresponding .hbs files
vmarta Aug 29, 2025
e1a940d
Fix import path
vmarta Aug 30, 2025
0436c03
Fix require path so it can work inside monorepo
vmarta Sep 2, 2025
0cf3273
Refactor to make it easier to get commerce agent config
vmarta Sep 2, 2025
83ec682
Fix module error
vmarta Sep 2, 2025
79c7bfb
Sync the eslint comments
vmarta Sep 2, 2025
946e287
Remove commented out code
vmarta Sep 2, 2025
385c8a1
Update CHANGELOG.md
vmarta Sep 2, 2025
4e5ace7
Merge branch 'develop' into vm/fix-config-parsing
vmarta Sep 2, 2025
5e503d3
Merge branch 'develop' into vm/fix-config-parsing
vcua-mobify Sep 3, 2025
c4ccb91
Merge branch 'develop' into vm/fix-config-parsing
vmarta Sep 3, 2025
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
16 changes: 12 additions & 4 deletions packages/pwa-kit-create-app/assets/bootstrap/js/config/default.js.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
/* eslint-disable @typescript-eslint/no-var-requires */
const sites = require('./sites.js')
const {parseCommerceAgentSettings} = require('./utils.js')
const {parseSettings} = require('./utils.js')

module.exports = {
app: {
Expand All @@ -23,9 +23,17 @@ module.exports = {
// Commerce shopping agent configuration for embedded messaging service
// This enables an agentic shopping experience in the application
// This property accepts either a JSON string or a plain JavaScript object.
// The value is set from the COMMERCE_AGENT_SETTINGS environment variable.
// If the COMMERCE_AGENT_SETTINGS environment variable is not set, the feature is disabled.
commerceAgent: parseCommerceAgentSettings(process.env.COMMERCE_AGENT_SETTINGS),
commerceAgent: parseSettings(process.env.COMMERCE_AGENT_SETTINGS) || {
enabled: 'false',
askAgentOnSearch: 'false',
embeddedServiceName: '',
embeddedServiceEndpoint: '',
scriptSourceUrl: '',
scrt2Url: '',
salesforceOrgId: '',
commerceOrgId: '',
siteId: ''
},
// Customize how your 'site' and 'locale' are displayed in the url.
url: {
{{#if answers.project.demo.enableDemoSettings}}
Expand Down
35 changes: 35 additions & 0 deletions packages/pwa-kit-create-app/assets/bootstrap/js/config/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

/**
* Safely parses settings from either a JSON string or object
* @param {string|object} settings - The settings
* @returns {object} Parsed settings object
*/
function parseSettings(settings) {
// If settings is already an object, return it
if (typeof settings === 'object' && settings !== null) {
return settings
}

// If settings is a string, try to parse it
if (typeof settings === 'string') {
try {
return JSON.parse(settings)
} catch (error) {
console.warn('Invalid json format:', error.message)
return
}
}

console.warn('Cannot parse settings from:', settings)
return
}

module.exports = {
parseSettings
}
48 changes: 0 additions & 48 deletions packages/pwa-kit-create-app/assets/bootstrap/js/config/utils.js.hbs
View file
Open in desktop

This file was deleted.

16 changes: 12 additions & 4 deletions ...es/pwa-kit-create-app/assets/templates/@salesforce/retail-react-app/config/default.js.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
/* eslint-disable @typescript-eslint/no-var-requires */
const sites = require('./sites.js')
const {parseCommerceAgentSettings} = require('./utils.js')
const {parseSettings} = require('./utils.js')

module.exports = {
app: {
Expand All @@ -23,9 +23,17 @@ module.exports = {
// Commerce shopping agent configuration for embedded messaging service
// This enables an agentic shopping experience in the application
// This property accepts either a JSON string or a plain JavaScript object.
// The value is set from the COMMERCE_AGENT_SETTINGS environment variable.
// If the COMMERCE_AGENT_SETTINGS environment variable is not set, the feature is disabled.
commerceAgent: parseCommerceAgentSettings(process.env.COMMERCE_AGENT_SETTINGS),
commerceAgent: parseSettings(process.env.COMMERCE_AGENT_SETTINGS) || {
enabled: 'false',
askAgentOnSearch: 'false',
embeddedServiceName: '',
embeddedServiceEndpoint: '',
scriptSourceUrl: '',
scrt2Url: '',
salesforceOrgId: '',
commerceOrgId: '',
siteId: ''
},
// Customize settings for your url
url: {
{{#if answers.project.demo.enableDemoSettings}}
Expand Down
35 changes: 35 additions & 0 deletions packages/pwa-kit-create-app/assets/templates/@salesforce/retail-react-app/config/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

/**
* Safely parses settings from either a JSON string or object
* @param {string|object} settings - The settings
* @returns {object} Parsed settings object
*/
function parseSettings(settings) {
// If settings is already an object, return it
if (typeof settings === 'object' && settings !== null) {
return settings
}

// If settings is a string, try to parse it
if (typeof settings === 'string') {
try {
return JSON.parse(settings)
} catch (error) {
console.warn('Invalid json format:', error.message)
return
}
}

console.warn('Cannot parse settings from:', settings)
return
}

module.exports = {
parseSettings
}
48 changes: 0 additions & 48 deletions ...ages/pwa-kit-create-app/assets/templates/@salesforce/retail-react-app/config/utils.js.hbs
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions packages/template-retail-react-app/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- [Bugfix] Pin `@chakra-ui/react` version to 2.7.0 to avoid breaking changes from 2.10.9 [#2658](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2658)
- Introduce optional prop `hybridAuthEnabled` to control Hybrid Auth specific behaviors in commerce-sdk-react [#3151](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3151)
- Inject sfdc_user_agent request header into all SCAPI requests for debugging and metrics prupose [#3183](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3183)
- Fix config parsing to gracefully handle missing properties [#3230](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3230)
Copy link
Contributor

@vcua-mobify vcua-mobify Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to add this same changelog entry to the pwa-kit-create-app changelog

Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided not to update the changelog for pwa-kit-create-app because the changes there are all related to shoppper agent, which has not been released yet. So we don't need to say we fix something that isn't released yet.

- [Bugfix] Fix unit test failures in generated projects [3204](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/3204)

## v7.0.0 (July 22, 2025)
Expand Down
5 changes: 3 additions & 2 deletions packages/template-retail-react-app/app/components/_app/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import {
import Seo from '@salesforce/retail-react-app/app/components/seo'
import ShopperAgent from '@salesforce/retail-react-app/app/components/shopper-agent'
import {getPathWithLocale} from '@salesforce/retail-react-app/app/utils/url'
import {getCommerceAgentConfig} from '@salesforce/retail-react-app/app/utils/config-utils'

const PlaceholderComponent = () => (
<Center p="2">
Expand Down Expand Up @@ -217,8 +218,8 @@ const App = (props) => {
}, [basket?.currency])

const commerceAgentConfiguration = useMemo(() => {
return config.app.commerceAgent
}, [config?.app])
return getCommerceAgentConfig()
}, [config.app.commerceAgent])

useEffect(() => {
// update the basket customer email
Expand Down
8 changes: 6 additions & 2 deletions packages/template-retail-react-app/app/components/search/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
categoryUrlBuilder
} from '@salesforce/retail-react-app/app/utils/url'
import {getConfig} from '@salesforce/pwa-kit-runtime/utils/ssr-config'
import {getCommerceAgentConfig} from '@salesforce/retail-react-app/app/utils/config-utils'

const onClient = typeof window !== 'undefined'

Expand Down Expand Up @@ -93,8 +94,11 @@ const formatSuggestions = (searchSuggestions, input) => {
*/
const Search = (props) => {
const config = getConfig()
const {enabled, askAgentOnSearch} = config.app.commerceAgent
const askAgentOnSearchEnabled = isAskAgentOnSearchEnabled(enabled, askAgentOnSearch)
const askAgentOnSearchEnabled = useMemo(() => {
Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the previous file (App component), we also call useMemo here.

const {enabled, askAgentOnSearch} = getCommerceAgentConfig()
return isAskAgentOnSearchEnabled(enabled, askAgentOnSearch)
}, [config.app.commerceAgent])

const [isOpen, setIsOpen] = useState(false)
const [searchQuery, setSearchQuery] = useState('')
const navigate = useNavigation()
Expand Down
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/hooks/use-einstein.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ const useEinstein = () => {
const {effectiveDnt} = useDNT()
const {getTokenWhenReady} = useAccessToken()
const {
app: {einsteinAPI: config}
Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked in the entire monorepo for any config parsing that may fail. This is one example where I tried to make sure there is no js undefined error.

app: {einsteinAPI: config = {}}
} = getConfig()
const {host, einsteinId, siteId, isProduction} = config

Expand Down
2 changes: 1 addition & 1 deletion packages/template-retail-react-app/app/pages/social-login-redirect/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const SocialLoginRedirect = () => {
const {data: customer} = useCurrentCustomer()
// Build redirectURI from config values
const appOrigin = useAppOrigin()
const redirectPath = getConfig().app.login.social?.redirectURI || ''
const redirectPath = getConfig().app.login?.social?.redirectURI || ''
Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app key in the config file already exists since the very beginning. BUT app.login may be missing from customers' projects.

const redirectURI = buildRedirectURI(appOrigin, redirectPath)

const locatedFrom = getSessionJSONItem('returnToPage')
Expand Down
12 changes: 10 additions & 2 deletions packages/template-retail-react-app/app/ssr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,16 @@ const throwSlasTokenValidationError = (message, code) => {
export const createRemoteJWKSet = (tenantId) => {
const appOrigin = getAppOrigin()
const {app: appConfig} = getConfig()
const shortCode = appConfig.commerceAPI.parameters.shortCode
const configTenantId = appConfig.commerceAPI.parameters.organizationId.replace(/^f_ecom_/, '')
const shortCode = appConfig.commerceAPI?.parameters?.shortCode
const configTenantId = appConfig.commerceAPI?.parameters?.organizationId?.replace(
/^f_ecom_/,
''
)
if (!shortCode || !configTenantId) {
throw new Error(
'Cannot find `commerceAPI.parameters.(shortCode|organizationId)` in your config file. Please check the config file.'
)
}
if (tenantId !== configTenantId) {
throw new Error(
`The tenant ID in your PWA Kit configuration ("${configTenantId}") does not match the tenant ID in the SLAS callback token ("${tenantId}").`
Expand Down
23 changes: 23 additions & 0 deletions packages/template-retail-react-app/app/utils/config-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import {getConfig} from '@salesforce/pwa-kit-runtime/utils/ssr-config'

export const getCommerceAgentConfig = () => {
const defaults = {
enabled: 'false',
askAgentOnSearch: 'false',
embeddedServiceName: '',
embeddedServiceEndpoint: '',
scriptSourceUrl: '',
scrt2Url: '',
salesforceOrgId: '',
commerceOrgId: '',
siteId: ''
}
return getConfig().app.commerceAgent ?? defaults
Comment on lines +11 to +22
Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If extensible project, it will automatically inherit this config-utils.js file.

Consider the scenario of the config not having commerceAgent property: calling getCommerceAgentConfig will return you the default values.

You may think that these defaults are redundant.. don't we already have them in the config file? Well yes, if you generate a new project. But existing config file may not have these values yet.

}
17 changes: 13 additions & 4 deletions packages/template-retail-react-app/config/default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,23 @@
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
// eslint-disable-next-line @typescript-eslint/no-var-requires
/* eslint-disable @typescript-eslint/no-var-requires */
const sites = require('./sites.js')
// eslint-disable-next-line @typescript-eslint/no-var-requires
const {parseCommerceAgentSettings} = require('./utils.js')
const {parseSettings} = require('./utils.js')

module.exports = {
app: {
commerceAgent: parseCommerceAgentSettings(process.env.COMMERCE_AGENT_SETTINGS),
commerceAgent: parseSettings(process.env.COMMERCE_AGENT_SETTINGS) || {
enabled: 'false',
askAgentOnSearch: 'false',
embeddedServiceName: '',
embeddedServiceEndpoint: '',
scriptSourceUrl: '',
scrt2Url: '',
salesforceOrgId: '',
commerceOrgId: '',
siteId: ''
},
Comment on lines +13 to +23
Copy link
Contributor Author

@vmarta vmarta Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a more generic parseSettings function. I think it'll be useful for future scenarios too.

And why explicitly have the default values here? To be consistent with the existing pattern seen with app.login. It's also good to be self-documenting as to what values go into commerceAgent.

url: {
site: 'path',
locale: 'path',
Expand Down
Loading
Loading