Skip to content

fix: better error handling in components #1642

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,18 @@ export default function EnvironmentOverview({
</div>
<div>
<div className="fs-13 fw-4 lh-20 cn-7 mb-4">Created by</div>
<div className="fs-13 fw-6 lh-20 cn-9 dc__word-break flexbox flex-align-center dc__gap-8">
<div
className="icon-dim-20 mw-20 flexbox flex-justify-center flex-align-center dc__border-radius-50-per dc__uppercase cn-0 fw-4"
style={{ backgroundColor: getRandomColor(appGroupListData.createdBy) }}
>
{appGroupListData.createdBy[0]}
</div>
{appGroupListData.createdBy}
</div>
{appGroupListData.createdBy
? (
<div className="fs-13 fw-6 lh-20 cn-9 dc__word-break flexbox flex-align-center dc__gap-8">
<div
className="icon-dim-20 mw-20 flexbox flex-justify-center flex-align-center dc__border-radius-50-per dc__uppercase cn-0 fw-4"
style={{ backgroundColor: getRandomColor(appGroupListData.createdBy) }}
>
{appGroupListData.createdBy[0]}
</div>
{appGroupListData.createdBy}
</div>
) : '-'}
</div>
</div>
</aside>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export default function EnvTriggerView({ filteredAppIds, isVirtualEnv }: AppGrou
setDefaultConfig(_isDefaultConfig)
setConfigPresent(isConfigPresent)
})
.catch()
Copy link
Member

Choose a reason for hiding this comment

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

Confirm with @arunjaindev

}

const preserveSelection = (_workflows: WorkflowType[]) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ export default function DeploymentTemplateOverrideForm({
const deploymentTemplateResp = isConfigProtectionEnabled
? await checkForProtectedLockedChanges()
: await api(+appId, +envId, payload)
if (deploymentTemplateResp.result.isLockConfigError && !saveEligibleChanges) {
if (deploymentTemplateResp.result?.isLockConfigError && !saveEligibleChanges) {
// checking if any locked changes and opening drawer to show eligible and locked ones
setLockedOverride(deploymentTemplateResp.result?.lockedOverride)
setDisableSaveEligibleChanges(deploymentTemplateResp.result?.disableSaveEligibleChanges)
setLockedOverride(deploymentTemplateResp.result.lockedOverride)
setDisableSaveEligibleChanges(deploymentTemplateResp.result.disableSaveEligibleChanges)
handleLockedDiffDrawer(true)
return
}
Expand All @@ -242,7 +242,7 @@ export default function DeploymentTemplateOverrideForm({

if (envOverrideValuesWithBasic) {
editorOnChange(YAML.stringify(envOverrideValuesWithBasic, { indent: 2 }), true)
} else {
} else if (deploymentTemplateResp.result?.envOverrideValues) {
dispatch({
type: DeploymentConfigStateActionTypes.tempFormData,
payload: YAML.stringify(deploymentTemplateResp.result.envOverrideValues),
Expand Down
1 change: 1 addition & 0 deletions src/components/app/details/appDetails/AppDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ export const Details: React.FC<DetailsType> = ({
}
},
)
.catch()
},
[
params.appId,
Expand Down
33 changes: 19 additions & 14 deletions src/components/app/details/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ export default function AppDetailsPage({ isV2 }: AppDetailsProps) {

const getSavedFilterData = async (groupId?: number): Promise<void> => {
setSelectedAppList([])
setSelectedGroupFilter([])
setAppListLoading(true)
setGroupFilterOptions([])
const { result } = await getEnvGroupList(+appId, FilterParentType.app)
if (result) {
try {
const { result } = await getEnvGroupList(+appId, FilterParentType.app)
if (result) {
const _groupFilterOption = []
let _selectedGroup
for (const group of result) {
Expand Down Expand Up @@ -106,26 +108,29 @@ export default function AppDetailsPage({ isV2 }: AppDetailsProps) {
}
_groupFilterOption.sort(sortOptionsByLabel)
setGroupFilterOptions(_groupFilterOption)
}
}
} catch {}
setAppListLoading(false)
}

const getAppListData = async (): Promise<void> => {
setSelectedAppList([])
setAppListLoading(true)
const { result } = await getAppOtherEnvironmentMin(appId)
if (result?.length) {
try {
const { result } = await getAppOtherEnvironmentMin(appId)
if (result?.length) {
setAppListOptions(
result
.map((app): OptionType => {
return {
value: `${app.environmentId}`,
label: app.environmentName,
}
})
.sort(sortOptionsByLabel),
result
.map((app): OptionType => {
return {
value: `${app.environmentId}`,
label: app.environmentName,
}
})
.sort(sortOptionsByLabel),
)
}
}
} catch {}
setAppListLoading(false)
}

Expand Down
1 change: 1 addition & 0 deletions src/components/app/details/triggerView/TriggerView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class TriggerView extends Component<TriggerViewProps, TriggerViewState> {
const _isDefaultConfig = response.result.is_default_configured
this.setState({ configs: isConfigPresent, isDefaultConfigPresent: _isDefaultConfig })
})
.catch()
}

getWorkflows = (isFromOnMount?: boolean) => {
Expand Down
28 changes: 1 addition & 27 deletions src/components/common/helpers/Helpers.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect, useCallback, useRef, useMemo, RefObject, useLayoutEffect } from 'react'
import React, { useState, useEffect, useCallback, useRef, useMemo } from 'react'
import {
showError,
useThrottledEffect,
Expand Down Expand Up @@ -1104,32 +1104,6 @@ export const reloadToastBody = () => {
)
}

export function useHeightObserver(callback): [RefObject<HTMLDivElement>] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The contents of the custom hook seem a little convoluted and unnecessary to me. Since this is (was) being used only in TerminalView, I have refactored it out. React warns us from using useLayoutEffect anyways. The motive was probably that the actually resizing should be differed to the next repaint to prevent infinite loops in ResizeObserver callback. We shouldn't change the div size inside the resizeObserver. Thus using window.requestAnimationFrame will defer the actual resize until the next repaint saving us from the infinite resize.

const ref = useRef(null)
const callbackRef = useRef(callback)

useLayoutEffect(() => {
callbackRef.current = callback
}, [callback])

const handleHeightChange = useCallback(() => {
callbackRef.current?.(ref.current.clientHeight)
}, [callbackRef])

useLayoutEffect(() => {
if (!ref.current) {
return
}
const observer = new ResizeObserver(handleHeightChange)
observer.observe(ref.current)
return () => {
observer.disconnect()
}
}, [handleHeightChange, ref])

return [ref]
}

export const getDeploymentAppType = (
allowedDeploymentTypes: DeploymentAppTypes[],
selectedDeploymentAppType: string,
Expand Down
12 changes: 6 additions & 6 deletions src/components/deploymentConfig/DeploymentConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ export default function DeploymentConfig({
const deploymentTemplateResp = isProtected
? await checkForProtectedLockedChanges()
: await getIfLockedConfigNonProtected(requestBody)
if (deploymentTemplateResp.result.isLockConfigError) {
setDisableSaveEligibleChanges(deploymentTemplateResp.result?.disableSaveEligibleChanges)
setLockedOverride(deploymentTemplateResp.result?.lockedOverride)
if (deploymentTemplateResp.result?.isLockConfigError) {
setDisableSaveEligibleChanges(deploymentTemplateResp.result.disableSaveEligibleChanges)
setLockedOverride(deploymentTemplateResp.result.lockedOverride)
handleLockedDiffDrawer(true)
return
}
Expand Down Expand Up @@ -566,9 +566,9 @@ export default function DeploymentConfig({
const requestBody = prepareDataToSave(true)
const api = state.chartConfig.id ? updateDeploymentTemplate : saveDeploymentTemplate
const deploymentTemplateResp = await api(requestBody, baseDeploymentAbortController.signal)
if (deploymentTemplateResp.result.isLockConfigError) {
setDisableSaveEligibleChanges(deploymentTemplateResp.result?.disableSaveEligibleChanges)
setLockedOverride(deploymentTemplateResp.result?.lockedOverride)
if (deploymentTemplateResp.result?.isLockConfigError) {
setDisableSaveEligibleChanges(deploymentTemplateResp.result.disableSaveEligibleChanges)
setLockedOverride(deploymentTemplateResp.result.lockedOverride)
handleLockedDiffDrawer(true)
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export default function DeploymentConfigFormCTA({
payload: true,
})
const deploymentTemplateResp = await checkForProtectedLockedChanges()
if (deploymentTemplateResp.result.isLockConfigError) {
if (deploymentTemplateResp.result?.isLockConfigError) {
setShowLockedDiffForApproval(true)
setLockedOverride(deploymentTemplateResp.result.lockedOverride)
handleLockedDiffDrawer(true)
Expand Down
1 change: 1 addition & 0 deletions src/components/login/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export default class Login extends Component<LoginProps, LoginFormState> {
loginList: list,
})
})
.catch()
if (typeof Storage !== 'undefined') {
if (localStorage.isDashboardAccessed) {
return
Expand Down
1 change: 1 addition & 0 deletions src/components/v2/appDetails/AppDetails.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ const AppDetailsComponent = ({
}
},
)
.catch()
}

const processDeploymentStatusData = (deploymentStatusDetailRes: DeploymentStatusDetailsType): void => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as XtermWebfont from 'xterm-webfont'
import SockJS from 'sockjs-client'
import moment from 'moment'
import CopyToast, { handleSelectionChange } from '../CopyToast'
import { elementDidMount, useHeightObserver } from '../../../../../../common/helpers/Helpers'
import { elementDidMount } from '../../../../../../common/helpers/Helpers'
import { CLUSTER_STATUS, SocketConnectionType } from '../../../../../../ClusterNodes/constants'
import { TERMINAL_STATUS } from './constants'
import './terminal.scss'
Expand All @@ -27,6 +27,7 @@ export default function TerminalView({
dataTestId,
}: TerminalViewType) {
const socket = useRef(null)
const termDiv = useRef(null)
const [firstMessageReceived, setFirstMessageReceived] = useState(false)
const [isReconnection, setIsReconnection] = useState(false)
const [popupText, setPopupText] = useState<boolean>(false)
Expand All @@ -42,7 +43,13 @@ export default function TerminalView({
}
}

const [myDivRef] = useHeightObserver(resizeSocket)
useEffect(() => {
/* requestAnimationFrame: will defer the resizeSocket callback to the next repaint;
* sparing us from - ResizeObserver loop completed with undelivered notifications */
const observer = new ResizeObserver(() => window.requestAnimationFrame(resizeSocket))
observer.observe(termDiv.current)
return () => observer.disconnect()
}, [])

useEffect(() => {
if (!terminalRef.current) {
Expand Down Expand Up @@ -240,7 +247,7 @@ export default function TerminalView({
<div className="terminal-wrapper" data-testid={dataTestId}>
{renderConnectionStrip()}
<div
ref={myDivRef}
ref={termDiv}
id="terminal-id"
data-testid="terminal-editor-container"
className="mt-8 mb-4 terminal-component ml-20"
Expand Down