-
-
Notifications
You must be signed in to change notification settings - Fork 734
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
fix: now null sdks will also be handled nicely #8984
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
@@ -325,7 +325,7 @@ export default class ClientApplicationsStore | |||
'COUNT(DISTINCT ci.instance_id) as unique_instance_count', | |||
), | |||
this.db.raw( | |||
'ARRAY_AGG(DISTINCT ci.sdk_version) as sdk_versions', | |||
'ARRAY_AGG(DISTINCT ci.sdk_version) FILTER (WHERE ci.sdk_version IS NOT NULL) as sdk_versions', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the sdk_version is null, do not add it to list.
@@ -394,7 +394,7 @@ export default class ClientApplicationsStore | |||
env = { | |||
name: environment, | |||
instanceCount: Number(unique_instance_count), | |||
sdks: sdk_versions, | |||
sdks: sdk_versions || [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If list is null, then use empty list.
@@ -24,6 +24,7 @@ export const applicationEnvironmentInstancesSchema = { | |||
}, | |||
sdkVersion: { | |||
type: 'string', | |||
nullable: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDK version can be null
@@ -412,7 +412,7 @@ export default class ClientApplicationsStore | |||
return acc; | |||
}, []); | |||
environments.forEach((env) => { | |||
env.sdks?.sort(); | |||
env.sdks.sort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to Server failed executing request: {} Cannot read properties of null (reading 'sort')","name":"controller/MetricsController","plan":"Enterprise","region":"eu-central-1","stack":"TypeError: Cannot read properties of null (reading 'sort')\n at /unleash/node_modules/unleash-server/dist/lib/db/client-applications-store.js:330:22\n at Array.forEach (<anonymous>)\n at ClientApplicationsStore.mapApplicationOverviewData (/unleash/node_modules/unleash-server/dist/lib/db/client-applications-store.js:329:22)\n at ClientApplicationsStore.getApplicationOverview (/unleash/node_modules/unleash-server/dist/lib/db/client-applications-store.js:291:21)
there seems to be some edge cases where this is still undefined
We had some error logs, that when SDK is null, then the application overview query was failing.
This solves it