Skip to content

Commit

Permalink
refactor(frontend): global error handler
Browse files Browse the repository at this point in the history
Motivation
----------
This will accomplish various things:

* Provide `toast` globally, so you can send success messages (e.g. copy to clipboard).
* Easier testing: Pass `toast` via dependency injection
* Consistency: Only one place to configure `toast`
* Code complexity: Remove unnecessarily nested try/catch
* Fix a todo: `console.error` in production and development

The major refactoring here is that we let errors propagate to the global
error handler. Everywhere where we used to call
`GlobalErrorHandler` the error was unexpected and we could not do
anything about it. In this case, it is best to `throw` properly and halt
the code execution.

How to test
-----------
1. `docker compose up`
2. `docker compose stop authentik`
3. Visit http://localhost:3000/app/signin
4. See error toast
  • Loading branch information
roschaefer committed Sep 17, 2024
1 parent 67f701f commit 9659142
Show file tree
Hide file tree
Showing 29 changed files with 156 additions and 172 deletions.
4 changes: 1 addition & 3 deletions frontend/(presenter)/pages/optin/+Page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ onBeforeMount(async () => {
try {
const result = await confirmNewsletterMutation({ code })
isError.value = !result?.data?.confirmNewsletter
} catch (error) {
// eslint-disable-next-line no-console
console.error(error)
} catch {
isError.value = true
}
Expand Down
3 changes: 3 additions & 0 deletions frontend/(presenter)/pages/optin/Page.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { confirmNewsletter } from '#presenter/graphql/mutations/confirmNewslette
import { vikePageContext } from '#renderer/context/usePageContext'
import i18n from '#renderer/plugins/i18n'
import { mockClient } from '#tests/mock.apolloClient'
import { createMockPlugin } from '#tests/plugin.globalErrorHandler'

import OptinPage from './+Page.vue'
import route from './+route'
Expand All @@ -17,6 +18,7 @@ vi.mock('vike/client/router')
vi.mocked(navigate).mockResolvedValue()

const confirmNewsletterMock = vi.fn()
const { mockPlugin } = createMockPlugin()

mockClient.setRequestHandler(
confirmNewsletter,
Expand All @@ -27,6 +29,7 @@ describe('OptinPage', () => {
const Wrapper = () => {
return mount(VApp, {
global: {
plugins: [mockPlugin],
provide: {
[vikePageContext as symbol]: {
routeParams: {
Expand Down
6 changes: 2 additions & 4 deletions frontend/app/components/buttons/LargeDreamMallButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@
<script setup lang="ts">
import { onMounted, ref } from 'vue'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
const buttonIsTurned = ref(false)
const warp = ref<HTMLInputElement | null>(null)
Expand Down Expand Up @@ -330,8 +328,8 @@ const onClick = (event: MouseEvent) => {
}
emit('click', 1)
} catch (error) {
GlobalErrorHandler.error('Error on CreateButton Click', error)
} catch (cause) {
throw new Error('Error on CreateButton Click', { cause })
}
}
Expand Down
5 changes: 2 additions & 3 deletions frontend/app/components/cockpit/about-me/AboutMe.vue
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
AddUserDetailInput,
AddSocialMediaInput,
} from '#app/stores/userStore'
import globalErrorHandler from '#renderer/plugins/globalErrorHandler'
import AboutMeView from './AboutMeView.vue'
import EditSocialMedia from './EditSocialMedia.vue'
Expand All @@ -72,8 +71,8 @@ const updateAvailability = async (newAvailability: UserAvailability) => {
name: user.value!.name,
availability: newAvailability,
})
} catch (error) {
globalErrorHandler.error(`Could not update user: ${(error as Error).message}`, error)
} catch (cause) {
throw new Error(`Could not update user: ${(cause as Error).message}`, { cause })
}
}
Expand Down
5 changes: 2 additions & 3 deletions frontend/app/components/cockpit/my-tables/TableItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import OptionButton from '#app/components/cockpit/options-list/OptionButton.vue'
import OptionsList from '#app/components/cockpit/options-list/OptionsList.vue'
import useModal from '#app/components/modal/useModal'
import { useTablesStore } from '#app/stores/tablesStore'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
import ChangeModerators from './change-moderators/ChangeModerators.vue'
import EditTable from './edit-table/EditTable.vue'
Expand Down Expand Up @@ -104,8 +103,8 @@ const deleteTable = async () => {
if (confirm(t('cockpit.myTables.deleteConfirmation'))) {
try {
await tablesStore.deleteTable(props.id)
} catch (error) {
GlobalErrorHandler.error('Could not delete table', error)
} catch (cause) {
throw new Error('Could not delete table', { cause })
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { reactive, ref } from 'vue'
import StepControl from '#app/components/steps/StepControl.vue'
import { Step } from '#app/components/steps/useSteps'
import { useTablesStore } from '#app/stores/tablesStore'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
import EnterNameAndVisibility from './EnterNameAndVisibility.vue'
import SelectUsers from './SelectUsers.vue'
Expand Down Expand Up @@ -73,8 +72,7 @@ const onSubmit = async () => {
)
if (!table) {
GlobalErrorHandler.error('Could not create table')
return
throw new Error('Could not create table')
}
createTableModel.tableId = table.id
Expand Down
5 changes: 2 additions & 3 deletions frontend/app/components/malltalk/settings/ChangeUsers.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import SimpleButton from '#app/components/buttons/SimpleButton.vue'
import UserSelection from '#app/components/user-selection/UserSelection.vue'
import { useTablesStore } from '#app/stores/tablesStore'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
import type MyTableSettings from '#app/components/malltalk/interfaces/MyTableSettings'
import type { StepEmits, StepProps } from '#app/components/steps/StepComponentTypes'
Expand All @@ -27,8 +26,8 @@ const onSubmitUsers = async (): Promise<string | void> => {
try {
await tablesStore.updateMyTableUsers(tableSettings.value.users)
emit('goTo', 'root')
} catch (error) {
GlobalErrorHandler.error('Error occurred by updating the users', error)
} catch (cause) {
throw new Error('Error occurred by updating the users', { cause })
}
}
</script>
10 changes: 5 additions & 5 deletions frontend/app/components/malltalk/settings/TableSettingsRoot.vue
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<script setup lang="ts">
import { navigate } from 'vike/client/router'
import { computed, ref } from 'vue'
import { computed, ref, inject } from 'vue'
import { useI18n } from 'vue-i18n'
import SimpleButton from '#app/components/buttons/SimpleButton.vue'
Expand All @@ -34,7 +34,10 @@ import { copyToClipboard } from '#app/utils/copyToClipboard'
import { usePageContext } from '#renderer/context/usePageContext'
import type { StepEmits, StepProps } from '#app/components/steps/StepComponentTypes'
import type { toast as Toast } from 'vue3-toastify'
const toast = inject<typeof Toast>('toast')
const copy = copyToClipboard(toast)
const { t } = useI18n()
defineProps<StepProps>()
Expand Down Expand Up @@ -68,10 +71,7 @@ const buttons = computed(() => [
indicator: copiedIndicator.value,
action: () => {
if (tableId.value) {
copyToClipboard(
tablesStore.getJoinTableUrl(tableId.value, META.BASE_URL),
t('globalErrorHandler.copiedToClipboard'),
)
copy(tablesStore.getJoinTableUrl(tableId.value, META.BASE_URL), t('copiedToClipboard'))
copiedIndicator.value = true
if (timerIndicator) clearTimeout(timerIndicator)
Expand Down
13 changes: 8 additions & 5 deletions frontend/app/components/malltalk/setup/SubmitTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,18 @@

<script lang="ts" setup>
import { navigate } from 'vike/client/router'
import { ref } from 'vue'
import { ref, inject } from 'vue'
import { useI18n } from 'vue-i18n'
import SimpleButton from '#app/components/buttons/SimpleButton.vue'
import LogoImage from '#app/components/menu/LogoImage.vue'
import { useTablesStore } from '#app/stores/tablesStore'
import { copyToClipboard } from '#app/utils/copyToClipboard'
import { usePageContext } from '#renderer/context/usePageContext'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
import type MyTableSettings from '#app/components/malltalk/interfaces/MyTableSettings'
import type { StepEmits, StepProps } from '#app/components/steps/StepComponentTypes'
import type { toast as Toast } from 'vue3-toastify'
defineProps<StepProps>()
const emit = defineEmits<StepEmits>()
Expand All @@ -70,8 +70,11 @@ const tableUrl = tablesStore.getJoinTableUrl(tableId, META.BASE_URL)
const copiedIndicator = ref(false)
let timerIndicator: ReturnType<typeof setTimeout> | null = null
const toast = inject<typeof Toast>('toast')
const copy = copyToClipboard(toast)
const copyUrl = () => {
copyToClipboard(tableUrl, t('globalErrorHandler.copiedToClipboard'))
copy(tableUrl, t('copiedToClipboard'))
copiedIndicator.value = true
if (timerIndicator) clearTimeout(timerIndicator)
Expand All @@ -86,8 +89,8 @@ const navigateToTable = async () => {
if (!tableId) return
try {
await navigate(tablesStore.getTableUri(tableId))
} catch (error) {
GlobalErrorHandler.error('Error opening table', error)
} catch (cause) {
throw new Error('Error opening table', { cause })
}
}
</script>
Expand Down
29 changes: 11 additions & 18 deletions frontend/app/components/malltalk/setup/TableSetup.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { useI18n } from 'vue-i18n'
import StepControl from '#app/components/steps/StepControl.vue'
import { Step } from '#app/components/steps/useSteps'
import { useTablesStore } from '#app/stores/tablesStore'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
import EnterNameAndVisibility from './EnterNameAndVisibility.vue'
import SelectUsers from './SelectUsers.vue'
Expand Down Expand Up @@ -85,24 +84,18 @@ defineEmits<{
}>()
const onSubmit = async () => {
try {
const table = await tablesStore.createMyTable(
tableSettings.name,
!tableSettings.isPrivate,
tableSettings.users,
)
if (!table) {
GlobalErrorHandler.error('Could not create MyTable')
return
}
const table = await tablesStore.createMyTable(
tableSettings.name,
!tableSettings.isPrivate,
tableSettings.users,
)
if (!table) {
throw new Error('Could not create MyTable')
}
tableSettings.tableId = await tablesStore.joinMyTable()
if (!tableSettings.tableId) {
GlobalErrorHandler.error('Could not join myTable')
}
} catch (error) {
GlobalErrorHandler.error('Error opening table', error)
tableSettings.tableId = await tablesStore.joinMyTable()
if (!tableSettings.tableId) {
throw new Error('Could not join myTable')
}
stepControl.value?.next()
Expand Down
5 changes: 2 additions & 3 deletions frontend/app/components/menu/UserDropdown.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@
import { inject } from 'vue'
import AuthService from '#app/services/AuthService'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
const authService = inject<AuthService>('authService')
async function signOut() {
try {
await authService?.signOut()
} catch (error) {
GlobalErrorHandler.error('auth error', error)
} catch (cause) {
throw new Error('auth error', { cause })
}
}
</script>
Expand Down
5 changes: 2 additions & 3 deletions frontend/app/components/user-selection/useSearchUsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useQuery } from '@vue/apollo-composable'
import { computed, ref } from 'vue'

import { searchUsersQuery } from '#app/graphql/queries/searchUsersQuery'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'

export type SearchUser = {
id: number
Expand Down Expand Up @@ -39,8 +38,8 @@ export default function useSearchUsers() {
searchString.value = query
try {
await refetch({ searchString: searchString.value })
} catch (e) {
GlobalErrorHandler.error('Error searching users:', e)
} catch (cause) {
throw new Error('Error searching users:', { cause })
}
}

Expand Down
16 changes: 8 additions & 8 deletions frontend/app/pages/auth/+Page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ import { navigate } from 'vike/client/router'
import { inject, onBeforeMount } from 'vue'
import AuthService from '#app/services/AuthService'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
const authService = inject<AuthService>('authService')
onBeforeMount(async () => {
let user
try {
const user = await authService?.signInCallback()
if (!user) {
throw new Error('Could not Sign In')
}
navigate('/app')
} catch (error) {
GlobalErrorHandler.error('auth error', error)
user = await authService?.signInCallback()
} catch (cause) {
throw new Error('auth error', { cause })
}
if (!user) {
throw new Error('Could not sign in')
}
navigate('/app')
})
</script>

Expand Down
19 changes: 9 additions & 10 deletions frontend/app/pages/auth/Page.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@ import { VApp } from 'vuetify/components'

import i18n from '#renderer/plugins/i18n'
import { authService } from '#tests/mock.authService'
import { errorHandlerSpy } from '#tests/plugin.globalErrorHandler'
import { createMockPlugin } from '#tests/plugin.globalErrorHandler'

import AuthPage from './+Page.vue'
import { title } from './+title'

vi.mock('vike/client/router')
vi.mocked(navigate).mockResolvedValue()

const { mockPlugin, errorSpy } = createMockPlugin()

describe('AuthPage', () => {
const Wrapper = () => {
return mount(VApp, {
global: { plugins: [mockPlugin] },
slots: {
default: h(AuthPage as Component),
},
Expand Down Expand Up @@ -70,28 +73,24 @@ describe('AuthPage', () => {
describe('no user returned', () => {
beforeEach(() => {
vi.clearAllMocks()
authServiceSpy.mockResolvedValue()
authServiceSpy.mockResolvedValue(undefined)
Wrapper()
})

it('throws an error', () => {
try {
wrapper = Wrapper()
} catch (err) {
// eslint-disable-next-line vitest/no-conditional-expect
expect(err).toBe('Could not Sign In')
}
expect(errorSpy).toHaveBeenCalledWith(new Error('Could not sign in'))
})
})

describe('signin callback with error', () => {
beforeEach(() => {
vi.clearAllMocks()
authServiceSpy.mockRejectedValue('Ouch!')
wrapper = Wrapper()
Wrapper()
})

it('logs the error on console', () => {
expect(errorHandlerSpy).toHaveBeenCalledWith('auth error', 'Ouch!')
expect(errorSpy).toHaveBeenCalledWith(new Error('auth error', { cause: 'Ouch!' }))
})
})
})
5 changes: 2 additions & 3 deletions frontend/app/pages/join-table/+Page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import { ref } from 'vue'
import MainButton from '#app/components/buttons/MainButton.vue'
import { joinTableAsGuestQuery } from '#app/graphql/queries/joinTableAsGuestQuery'
import { usePageContext } from '#renderer/context/usePageContext'
import GlobalErrorHandler from '#renderer/plugins/globalErrorHandler'
const pageContext = usePageContext()
const tableId = Number(pageContext.routeParams?.id)
Expand Down Expand Up @@ -68,8 +67,8 @@ const getTableLink = async () => {
if (joinTableAsGuestQueryResult.value) {
window.location.href = joinTableAsGuestQueryResult.value.joinTableAsGuest
}
} catch (error) {
GlobalErrorHandler.error('table link not found', error)
} catch (cause) {
throw new Error('table link not found', { cause })
}
}
</script>
Loading

0 comments on commit 9659142

Please sign in to comment.