Skip to content

Commit

Permalink
Fix error modal handling
Browse files Browse the repository at this point in the history
As discussed, this is an alternate approach to #174 for fixing #168. It handles errors directly in `withLoading` and `pactaClient`. The problem then is that both these functions return _something_, so you still have to throw and interrupt the flow of code (I originally tried just leaving hanging promises, but our linter informed me that's a really bad idea)

Since we have to throw, we throw a sentinel so that anything further up the chain knows not to render another dialog.

I tested it in a few different cases and confirmed it does what we expect.
  • Loading branch information
bcspragu committed Jan 24, 2024
1 parent b4a5e27 commit dff9f3d
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 10 deletions.
9 changes: 6 additions & 3 deletions frontend/app.vue
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<script setup lang="ts">
import { isSilent } from '~/lib/error'
import { serializeError } from 'serialize-error'
const { loading: { clearLoading }, error: { errorModalVisible, error } } = useModal()
Expand All @@ -7,9 +8,11 @@ const handleError = (err: Error) => {
if (process.client) {
console.log(err)
}
error.value = serializeError(err)
errorModalVisible.value = true
clearLoading()
if (!isSilent(err)) {
error.value = serializeError(err)
errorModalVisible.value = true
clearLoading()
}
}
onErrorCaptured((err: unknown, _instance: ComponentPublicInstance | null, _info: string) => {
Expand Down
21 changes: 16 additions & 5 deletions frontend/composables/useModal.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { type Ref } from 'vue'
import { type ErrorObject } from 'serialize-error'
import { type ErrorObject, serializeError } from 'serialize-error'
import { createErrorWithRemediation, isSilent, Remediation } from '~/lib/error'

export const useModal = () => {
const prefix = 'useModal'
Expand Down Expand Up @@ -35,11 +36,21 @@ export const useModal = () => {
return () => loadingSet.value.delete(loadKey)
}
const clearLoading = () => { loadingSet.value.clear() }
const withLoading = async <T> (fn: () => Promise<T>, opKey: string): (Promise<T>) => {
const withLoading = <T> (fn: () => Promise<T>, opKey: string): Promise<T> => {
startLoading(opKey)
const p = fn()
void p.finally(stopLoading(opKey))
return await p
return fn()
.catch((e: unknown) => {
if (!isSilent(e)) {
error.value = serializeError(e)
errorModalVisible.value = true
clearLoading()
}

throw createErrorWithRemediation(`withLoading: ${opKey}`, Remediation.Silent)
})
.finally(() => {
stopLoading(opKey)
})
}
const onMountedWithLoading = (fn: () => void, opKey: string) => {
startLoading(opKey)
Expand Down
20 changes: 19 additions & 1 deletion frontend/composables/usePACTA.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { BaseHttpRequest } from '~/openapi/generated/pacta/core/BaseHttpRequest'
import { CancelablePromise } from '~/openapi/generated/pacta/core/CancelablePromise'
import type { OpenAPIConfig } from '~/openapi/generated/pacta/core/OpenAPI'
import { request as __request } from '~/openapi/generated/pacta/core/request'
import { serializeError } from 'serialize-error'
import { createErrorWithRemediation, isSilent, Remediation } from '~/lib/error'

export const usePACTA = () => {
const {
Expand All @@ -12,6 +14,8 @@ export const usePACTA = () => {
pactaClientWithHttpRequestClass, // On the client, wrap with a check for a fresh cookie.
} = useAPI()

const { loading: { clearLoading }, error: { errorModalVisible, error } } = useModal()

if (process.server) {
const jwt = useCookie('jwt')
if (jwt.value) {
Expand Down Expand Up @@ -54,7 +58,21 @@ export const usePACTA = () => {
return cancelablePromise
})
.then(resolve)
.catch(reject)
.catch((e: unknown) => {
// We know we're at the "bottom" of the call chain here in usePACTA
// and so nothing will be sending a silent error, but we don't know
// how the code will change in the future, so we check just to be
// safe, lest we accidentally overlay multiple error modals with
// less useful information.
if (!isSilent(e)) {
error.value = serializeError(e)
errorModalVisible.value = true
clearLoading()
}

const err = createErrorWithRemediation(`${options.method} ${options.url} failed`, Remediation.Silent)
reject(err)
})
})
}
}
Expand Down
25 changes: 25 additions & 0 deletions frontend/lib/error/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export enum Remediation {
Reload = 'reload',
FileBug = 'file-bug',
CheckUrl = 'check-url',
Silent = 'silent',
}

interface RemediationData {
Expand All @@ -18,6 +19,30 @@ interface PACTAError extends NuxtError {
data: PACTAErrorData
}

export const isSilent = (err: any): boolean => {
if (!('data' in err)) {
return false
}

if (typeof (err.data) !== 'object' || err.data === null) {
return false
}

if (!('type' in err.data) || typeof (err.data.type) !== 'string') {
return false
}

if (err.data.type !== 'remediation') {
return false
}

if (!('remediation' in err.data) || typeof (err.data.remediation) !== 'string') {
return false
}

return err.data.remediation === Remediation.Silent
}

export const createErrorWithRemediation = (err: string | Partial<NuxtError>, r: Remediation): PACTAError => {
console.log(err)
const nuxtErr = createError(err)
Expand Down
1 change: 0 additions & 1 deletion frontend/pages/admin/merge.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const fromUserId = useState<string>(`${prefix}.fromUserId`, () => '')
const toUserId = useState<string>(`${prefix}.toUserId`, () => '')
const done = useState<boolean>(`${prefix}.done`, () => false)
// TODO(#168) An example of the error here.
const doMerge = (): void => {
void withLoading(() => pactaClient.mergeUsers({
fromUserId: fromUserId.value,
Expand Down

0 comments on commit dff9f3d

Please sign in to comment.