internal: Add lints for promises (#3733)

* Add lints for floating and misued promises

* Add logging async errors in main

* Add async error catch in test-utils

* Change any to unknown

* Trap promise errors and ignore more await warnings

* Add more ignores and toSync helper

* Fix more lint warnings

* Add more ignores and fixes

* Add more reject reporting

* Add accepting arbitrary parameters to toSync()

* Fix more lints

* Revert unintentional change to non-arrow function

* Revert unintentional change to use arrow function

* Fix new warnings in main with auto updater

* Fix formatting

* Change lints to error

This is what the recommended type checked rules do.

* Fix to properly report promise rejections

* Fix formatting

* Fix formatting

* Remove unused import

* Remove unused convenience function

* Move type helpers

* Fix to not return promise when caller doesn't expect it

* Add ignores to lsp code
This commit is contained in:
Jonathan Tran
2024-09-09 18:17:45 -04:00
committed by GitHub
parent 0a72d7a39a
commit 25443eba31
59 changed files with 786 additions and 528 deletions

View File

@ -151,6 +151,7 @@ export function useCalc({
})
if (trap(error)) return
}
// eslint-disable-next-line @typescript-eslint/no-floating-promises
executeAst({
ast,
engineCommandManager,

View File

@ -2,6 +2,7 @@ import { useState, useEffect } from 'react'
import { EngineCommandManagerEvents } from 'lang/std/engineConnection'
import { engineCommandManager, sceneInfra } from 'lib/singletons'
import { throttle, isReducedMotion } from 'lib/utils'
import { reportRejection } from 'lib/trap'
const updateDollyZoom = throttle(
(newFov: number) => sceneInfra.camControls.dollyZoom(newFov),
@ -16,8 +17,8 @@ export const CamToggle = () => {
useEffect(() => {
engineCommandManager.addEventListener(
EngineCommandManagerEvents.SceneReady,
async () => {
sceneInfra.camControls.dollyZoom(fov)
() => {
sceneInfra.camControls.dollyZoom(fov).catch(reportRejection)
}
)
}, [])
@ -26,11 +27,11 @@ export const CamToggle = () => {
if (isPerspective) {
isReducedMotion()
? sceneInfra.camControls.useOrthographicCamera()
: sceneInfra.camControls.animateToOrthographic()
: sceneInfra.camControls.animateToOrthographic().catch(reportRejection)
} else {
isReducedMotion()
? sceneInfra.camControls.usePerspectiveCamera()
: sceneInfra.camControls.animateToPerspective()
? sceneInfra.camControls.usePerspectiveCamera().catch(reportRejection)
: sceneInfra.camControls.animateToPerspective().catch(reportRejection)
}
setIsPerspective(!isPerspective)
}

View File

@ -1,5 +1,6 @@
import { CommandLog } from 'lang/std/engineConnection'
import { engineCommandManager } from 'lib/singletons'
import { reportRejection } from 'lib/trap'
import { useState, useEffect } from 'react'
export function useEngineCommands(): [CommandLog[], () => void] {
@ -77,9 +78,11 @@ export const EngineCommands = () => {
/>
<button
data-testid="custom-cmd-send-button"
onClick={() =>
engineCommandManager.sendSceneCommand(JSON.parse(customCmd))
}
onClick={() => {
engineCommandManager
.sendSceneCommand(JSON.parse(customCmd))
.catch(reportRejection)
}}
>
Send custom command
</button>

View File

@ -176,9 +176,11 @@ const FileTreeItem = ({
`import("${fileOrDir.path.replace(project.path, '.')}")\n` +
codeManager.code
)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
codeManager.writeToFile()
// Prevent seeing the model built one piece at a time when changing files
// eslint-disable-next-line @typescript-eslint/no-floating-promises
kclManager.executeCode(true)
} else {
// Let the lsp servers know we closed a file.
@ -388,14 +390,14 @@ interface FileTreeProps {
export const FileTreeMenu = () => {
const { send } = useFileContext()
async function createFile() {
function createFile() {
send({
type: 'Create file',
data: { name: '', makeDir: false },
})
}
async function createFolder() {
function createFolder() {
send({
type: 'Create file',
data: { name: '', makeDir: true },

View File

@ -27,6 +27,7 @@ import {
} from './ContextMenu'
import { Popover } from '@headlessui/react'
import { CustomIcon } from './CustomIcon'
import { reportRejection } from 'lib/trap'
const CANVAS_SIZE = 80
const FRUSTUM_SIZE = 0.5
@ -67,7 +68,9 @@ export default function Gizmo() {
<ContextMenuItem
key={axisName}
onClick={() => {
sceneInfra.camControls.updateCameraToAxis(axisName as AxisNames)
sceneInfra.camControls
.updateCameraToAxis(axisName as AxisNames)
.catch(reportRejection)
}}
>
{axisSemantic} view
@ -75,7 +78,7 @@ export default function Gizmo() {
)),
<ContextMenuItem
onClick={() => {
sceneInfra.camControls.resetCameraPosition()
sceneInfra.camControls.resetCameraPosition().catch(reportRejection)
}}
>
Reset view
@ -299,7 +302,7 @@ const initializeMouseEvents = (
const handleClick = () => {
if (raycasterIntersect.current) {
const axisName = raycasterIntersect.current.object.name as AxisNames
sceneInfra.camControls.updateCameraToAxis(axisName)
sceneInfra.camControls.updateCameraToAxis(axisName).catch(reportRejection)
}
}

View File

@ -8,6 +8,7 @@ import { createAndOpenNewProject } from 'lib/desktopFS'
import { useAbsoluteFilePath } from 'hooks/useAbsoluteFilePath'
import { useLspContext } from './LspProvider'
import { openExternalBrowserIfDesktop } from 'lib/openWindow'
import { reportRejection } from 'lib/trap'
const HelpMenuDivider = () => (
<div className="h-[1px] bg-chalkboard-110 dark:bg-chalkboard-80" />
@ -115,7 +116,9 @@ export function HelpMenu(props: React.PropsWithChildren) {
if (isInProject) {
navigate(filePath + PATHS.ONBOARDING.INDEX)
} else {
createAndOpenNewProject({ onProjectOpen, navigate })
createAndOpenNewProject({ onProjectOpen, navigate }).catch(
reportRejection
)
}
}}
>

View File

@ -12,6 +12,7 @@ import { CoreDumpManager } from 'lib/coredump'
import openWindow, { openExternalBrowserIfDesktop } from 'lib/openWindow'
import { NetworkMachineIndicator } from './NetworkMachineIndicator'
import { ModelStateIndicator } from './ModelStateIndicator'
import { reportRejection } from 'lib/trap'
export function LowerRightControls({
children,
@ -25,7 +26,7 @@ export function LowerRightControls({
const linkOverrideClassName =
'!text-chalkboard-70 hover:!text-chalkboard-80 dark:!text-chalkboard-40 dark:hover:!text-chalkboard-30'
async function reportbug(event: {
function reportbug(event: {
preventDefault: () => void
stopPropagation: () => void
}) {
@ -34,7 +35,9 @@ export function LowerRightControls({
if (!coreDumpManager) {
// open default reporting option
openWindow('https://github.com/KittyCAD/modeling-app/issues/new/choose')
openWindow(
'https://github.com/KittyCAD/modeling-app/issues/new/choose'
).catch(reportRejection)
} else {
toast
.promise(
@ -56,7 +59,7 @@ export function LowerRightControls({
if (err) {
openWindow(
'https://github.com/KittyCAD/modeling-app/issues/new/choose'
)
).catch(reportRejection)
}
})
}

View File

@ -160,7 +160,9 @@ export const LspProvider = ({ children }: { children: React.ReactNode }) => {
// Update the folding ranges, since the AST has changed.
// This is a hack since codemirror does not support async foldService.
// When they do we can delete this.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
plugin.updateFoldingRanges()
// eslint-disable-next-line @typescript-eslint/no-floating-promises
plugin.requestSemanticTokens()
break
case 'kcl/memoryUpdated':

View File

@ -73,7 +73,7 @@ import { EditorSelection, Transaction } from '@codemirror/state'
import { useNavigate, useSearchParams } from 'react-router-dom'
import { letEngineAnimateAndSyncCamAfter } from 'clientSideScene/CameraControls'
import { getVarNameModal } from 'hooks/useToolbarGuards'
import { err, trap } from 'lib/trap'
import { err, reportRejection, trap } from 'lib/trap'
import { useCommandsContext } from 'hooks/useCommandsContext'
import { modelingMachineEvent } from 'editor/manager'
import { hasValidFilletSelection } from 'lang/modifyAst/addFillet'
@ -152,14 +152,17 @@ export const ModelingMachineProvider = ({
store.videoElement?.pause()
kclManager.executeCode().then(() => {
if (engineCommandManager.engineConnection?.idleMode) return
kclManager
.executeCode()
.then(() => {
if (engineCommandManager.engineConnection?.idleMode) return
store.videoElement?.play().catch((e) => {
console.warn('Video playing was prevented', e)
store.videoElement?.play().catch((e) => {
console.warn('Video playing was prevented', e)
})
})
})
})()
.catch(reportRejection)
})().catch(reportRejection)
},
'Set mouse state': assign(({ context, event }) => {
if (event.type !== 'Set mouse state') return {}
@ -316,9 +319,10 @@ export const ModelingMachineProvider = ({
})
codeMirrorSelection && dispatchSelection(codeMirrorSelection)
engineEvents &&
engineEvents.forEach((event) =>
engineEvents.forEach((event) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
engineCommandManager.sendSceneCommand(event)
)
})
updateSceneObjectColors()
return {
@ -349,9 +353,10 @@ export const ModelingMachineProvider = ({
selections,
})
engineEvents &&
engineEvents.forEach((event) =>
engineEvents.forEach((event) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
engineCommandManager.sendSceneCommand(event)
)
})
updateSceneObjectColors()
return {
selectionRanges: selections,
@ -378,7 +383,7 @@ export const ModelingMachineProvider = ({
return {}
}
),
Make: async ({ event }) => {
Make: ({ event }) => {
if (event.type !== 'Make') return
// Check if we already have an export intent.
if (engineCommandManager.exportIntent) {
@ -410,19 +415,21 @@ export const ModelingMachineProvider = ({
}
// Artificially delay the export in playwright tests
toast.promise(
exportFromEngine({
format: format,
}),
toast
.promise(
exportFromEngine({
format: format,
}),
{
loading: 'Starting print...',
success: 'Started print successfully',
error: 'Error while starting print',
}
)
{
loading: 'Starting print...',
success: 'Started print successfully',
error: 'Error while starting print',
}
)
.catch(reportRejection)
},
'Engine export': async ({ event }) => {
'Engine export': ({ event }) => {
if (event.type !== 'Export') return
if (engineCommandManager.exportIntent) {
toast.error('Already exporting')
@ -474,23 +481,25 @@ export const ModelingMachineProvider = ({
format.selection = { type: 'default_scene' }
}
toast.promise(
exportFromEngine({
format: format as Models['OutputFormat_type'],
}),
{
loading: 'Exporting...',
success: 'Exported successfully',
error: 'Error while exporting',
}
)
toast
.promise(
exportFromEngine({
format: format as Models['OutputFormat_type'],
}),
{
loading: 'Exporting...',
success: 'Exported successfully',
error: 'Error while exporting',
}
)
.catch(reportRejection)
},
'Submit to Text-to-CAD API': async ({ event }) => {
'Submit to Text-to-CAD API': ({ event }) => {
if (event.type !== 'Text-to-CAD') return
const trimmedPrompt = event.data.prompt.trim()
if (!trimmedPrompt) return
void submitAndAwaitTextToKcl({
submitAndAwaitTextToKcl({
trimmedPrompt,
fileMachineSend,
navigate,
@ -501,7 +510,7 @@ export const ModelingMachineProvider = ({
theme: theme.current,
highlightEdges: highlightEdges.current,
},
})
}).catch(reportRejection)
},
},
guards: {

View File

@ -8,6 +8,7 @@ import { editorShortcutMeta } from './KclEditorPane'
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
import { kclManager } from 'lib/singletons'
import { openExternalBrowserIfDesktop } from 'lib/openWindow'
import { reportRejection } from 'lib/trap'
export const KclEditorMenu = ({ children }: PropsWithChildren) => {
const { enable: convertToVarEnabled, handleClick: handleConvertToVarClick } =
@ -47,7 +48,9 @@ export const KclEditorMenu = ({ children }: PropsWithChildren) => {
{convertToVarEnabled && (
<Menu.Item>
<button
onClick={() => handleConvertToVarClick()}
onClick={() => {
handleConvertToVarClick().catch(reportRejection)
}}
className={styles.button}
>
<span>Convert to Variable</span>

View File

@ -57,6 +57,7 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
icon: 'printer3d',
iconClassName: '!p-0',
keybinding: 'Ctrl + Shift + M',
// eslint-disable-next-line @typescript-eslint/no-misused-promises
action: async () => {
commandBarSend({
type: 'Find and select command',

View File

@ -4,6 +4,8 @@ import Tooltip from './Tooltip'
import { ConnectingTypeGroup } from '../lang/std/engineConnection'
import { useNetworkContext } from '../hooks/useNetworkContext'
import { NetworkHealthState } from '../hooks/useNetworkStatus'
import { toSync } from 'lib/utils'
import { reportRejection } from 'lib/trap'
export const NETWORK_HEALTH_TEXT: Record<NetworkHealthState, string> = {
[NetworkHealthState.Ok]: 'Connected',
@ -160,13 +162,13 @@ export const NetworkHealthIndicator = () => {
</div>
{issues[name as ConnectingTypeGroup] && (
<button
onClick={async () => {
onClick={toSync(async () => {
await navigator.clipboard.writeText(
JSON.stringify(error, null, 2) || ''
)
setHasCopied(true)
setTimeout(() => setHasCopied(false), 5000)
}}
}, reportRejection)}
className="flex w-fit gap-2 items-center bg-transparent text-sm p-1 py-0 my-0 -mx-1 text-destroy-80 dark:text-destroy-10 hover:bg-transparent border-transparent dark:border-transparent hover:border-destroy-80 dark:hover:border-destroy-80 dark:hover:bg-destroy-80"
>
{hasCopied ? 'Copied' : 'Copy Error'}

View File

@ -8,6 +8,8 @@ import Tooltip from '../Tooltip'
import { DeleteConfirmationDialog } from './DeleteProjectDialog'
import { ProjectCardRenameForm } from './ProjectCardRenameForm'
import { Project } from 'lib/project'
import { toSync } from 'lib/utils'
import { reportRejection } from 'lib/trap'
function ProjectCard({
project,
@ -165,10 +167,10 @@ function ProjectCard({
{isConfirmingDelete && (
<DeleteConfirmationDialog
title="Delete Project"
onConfirm={async () => {
onConfirm={toSync(async () => {
await handleDeleteProject(project)
setIsConfirmingDelete(false)
}}
}, reportRejection)}
onDismiss={() => setIsConfirmingDelete(false)}
>
<p className="my-4">

View File

@ -6,6 +6,8 @@ import React, { useMemo } from 'react'
import toast from 'react-hot-toast'
import Tooltip from './Tooltip'
import { useSettingsAuthContext } from 'hooks/useSettingsAuthContext'
import { reportRejection } from 'lib/trap'
import { toSync } from 'lib/utils'
export const RefreshButton = ({ children }: React.PropsWithChildren) => {
const { auth } = useSettingsAuthContext()
@ -50,11 +52,12 @@ export const RefreshButton = ({ children }: React.PropsWithChildren) => {
// Window may not be available in some environments
window?.location.reload()
})
.catch(reportRejection)
}
return (
<button
onClick={refresh}
onClick={toSync(refresh, reportRejection)}
className="p-1 m-0 bg-chalkboard-10/80 dark:bg-chalkboard-100/50 hover:bg-chalkboard-10 dark:hover:bg-chalkboard-100 rounded-full border border-solid border-chalkboard-20 dark:border-chalkboard-90"
>
<CustomIcon name="exclamationMark" className="w-5 h-5" />

View File

@ -20,6 +20,8 @@ import { createAndOpenNewProject, getSettingsFolderPaths } from 'lib/desktopFS'
import { useDotDotSlash } from 'hooks/useDotDotSlash'
import { ForwardedRef, forwardRef, useEffect } from 'react'
import { useLspContext } from 'components/LspProvider'
import { toSync } from 'lib/utils'
import { reportRejection } from 'lib/trap'
interface AllSettingsFieldsProps {
searchParamTab: SettingsLevel
@ -54,7 +56,7 @@ export const AllSettingsFields = forwardRef(
)
: undefined
async function restartOnboarding() {
function restartOnboarding() {
send({
type: `set.app.onboardingStatus`,
data: { level: 'user', value: '' },
@ -82,6 +84,7 @@ export const AllSettingsFields = forwardRef(
}
}
}
// eslint-disable-next-line @typescript-eslint/no-floating-promises
navigateToOnboardingStart()
}, [isFileSettings, navigate, state])
@ -190,7 +193,7 @@ export const AllSettingsFields = forwardRef(
{isDesktop() && (
<ActionButton
Element="button"
onClick={async () => {
onClick={toSync(async () => {
const paths = await getSettingsFolderPaths(
projectPath ? decodeURIComponent(projectPath) : undefined
)
@ -199,7 +202,7 @@ export const AllSettingsFields = forwardRef(
return new Error('finalPath undefined')
}
window.electron.showInFolder(finalPath)
}}
}, reportRejection)}
iconStart={{
icon: 'folder',
size: 'sm',
@ -211,14 +214,14 @@ export const AllSettingsFields = forwardRef(
)}
<ActionButton
Element="button"
onClick={async () => {
onClick={toSync(async () => {
const defaultDirectory = await getInitialDefaultDir()
send({
type: 'Reset settings',
defaultDirectory,
})
toast.success('Settings restored to default')
}}
}, reportRejection)}
iconStart={{
icon: 'refresh',
size: 'sm',

View File

@ -108,6 +108,7 @@ export const SettingsAuthProviderBase = ({
sceneInfra.baseUnit = newBaseUnit
},
setEngineTheme: ({ context }) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
engineCommandManager.sendSceneCommand({
cmd_id: uuidv4(),
type: 'modeling_cmd_req',
@ -118,6 +119,7 @@ export const SettingsAuthProviderBase = ({
})
const opposingTheme = getOppositeTheme(context.app.theme.current)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
engineCommandManager.sendSceneCommand({
cmd_id: uuidv4(),
type: 'modeling_cmd_req',
@ -137,6 +139,7 @@ export const SettingsAuthProviderBase = ({
sceneInfra.theme = opposingTheme
},
setEngineEdges: ({ context }) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
engineCommandManager.sendSceneCommand({
cmd_id: uuidv4(),
type: 'modeling_cmd_req',
@ -186,6 +189,7 @@ export const SettingsAuthProviderBase = ({
resetSettingsIncludesUnitChange
) {
// Unit changes requires a re-exec of code
// eslint-disable-next-line @typescript-eslint/no-floating-promises
kclManager.executeCode(true)
} else {
// For any future logging we'd like to do
@ -197,8 +201,10 @@ export const SettingsAuthProviderBase = ({
console.error('Error executing AST after settings change', e)
}
},
persistSettings: ({ context }) =>
saveSettings(context, loadedProject?.project?.path),
persistSettings: ({ context }) => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
saveSettings(context, loadedProject?.project?.path)
},
},
}),
{ input: loadedSettings }
@ -289,6 +295,7 @@ export const SettingsAuthProviderBase = ({
actions: {
goToSignInPage: () => {
navigate(PATHS.SIGN_IN)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
logout()
},
goToIndexPage: () => {
@ -330,13 +337,11 @@ export const SettingsAuthProviderBase = ({
export default SettingsAuthProvider
export function logout() {
export async function logout() {
localStorage.removeItem(TOKEN_PERSIST_KEY)
return (
!isDesktop() &&
fetch(withBaseUrl('/logout'), {
method: 'POST',
credentials: 'include',
})
)
if (isDesktop()) return Promise.resolve(null)
return fetch(withBaseUrl('/logout'), {
method: 'POST',
credentials: 'include',
})
}

View File

@ -53,9 +53,10 @@ export const Stream = () => {
* executed. If we can find a way to do this from a more
* central place, we can move this code there.
*/
async function executeCodeAndPlayStream() {
kclManager.executeCode(true).then(() => {
videoRef.current?.play().catch((e) => {
function executeCodeAndPlayStream() {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
kclManager.executeCode(true).then(async () => {
await videoRef.current?.play().catch((e) => {
console.warn('Video playing was prevented', e, videoRef.current)
})
setStreamState(StreamState.Playing)
@ -218,12 +219,12 @@ export const Stream = () => {
*/
useEffect(() => {
if (!kclManager.isExecuting) {
setTimeout(() =>
setTimeout(() => {
// execute in the next event loop
videoRef.current?.play().catch((e) => {
console.warn('Video playing was prevented', e, videoRef.current)
})
)
})
}
}, [kclManager.isExecuting])
@ -290,6 +291,7 @@ export const Stream = () => {
if (state.matches({ idle: 'showPlanes' })) return
if (!context.store?.didDragInStream && btnName(e).left) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
sendSelectEventToEngine(
e,
videoRef.current,

View File

@ -28,6 +28,7 @@ import { ActionButton } from './ActionButton'
import { commandBarMachine } from 'machines/commandBarMachine'
import { EventFrom } from 'xstate'
import { fileMachine } from 'machines/fileMachine'
import { reportRejection } from 'lib/trap'
const CANVAS_SIZE = 128
const PROMPT_TRUNCATE_LENGTH = 128
@ -297,7 +298,7 @@ export function ToastTextToCadSuccess({
name={hasCopied ? 'Close' : 'Reject'}
onClick={() => {
if (!hasCopied) {
sendTelemetry(modelId, 'rejected', token)
sendTelemetry(modelId, 'rejected', token).catch(reportRejection)
}
if (isDesktop()) {
// Delete the file from the project
@ -323,6 +324,7 @@ export function ToastTextToCadSuccess({
}}
name="Accept"
onClick={() => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
sendTelemetry(modelId, 'accepted', token)
navigate(
`${PATHS.FILE}/${encodeURIComponent(
@ -342,7 +344,9 @@ export function ToastTextToCadSuccess({
}}
name="Copy to clipboard"
onClick={() => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
sendTelemetry(modelId, 'accepted', token)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
navigator.clipboard.writeText(data.code || '// no code found')
setShowCopiedUi(true)
setHasCopied(true)