Address review comments

frontend/app.vue

@@ -1,5 +1,5 @@
 <script setup lang="ts">
-const { loading: { onMountedWithLoading, clearLoading }, error: { errorModalVisible, error } } = useModal()
+const { loading: { clearLoading }, error: { errorModalVisible, error } } = useModal()
 
 const handleError = (err: Error) => {
   error.value = err
@@ -19,8 +19,6 @@ onErrorCaptured((err: unknown, _instance: ComponentPublicInstance | null, _info:
   handleError(error)
   return false // Don't propagate
 })
-
-onMountedWithLoading(() => { /* nothing to do */ }, 'defaultLayout.onMountedWithLoading')
 </script>
 
 <template>

frontend/components/modal/Error.vue

@@ -1,32 +1,27 @@
 <script setup lang="ts">
-import { watch } from 'vue'
-
 interface Props {
-  isFullPage?: boolean
+  routeBackOnClose?: boolean
 }
 
 const props = withDefaults(defineProps<Props>(), {
-  isFullPage: false
+  routeBackOnClose: false
 })
 
 const { error: { errorModalVisible, error: modalError } } = useModal()
 const error = useError()
 const router = useRouter()
 
-watch(errorModalVisible, async (newV, oldV) => {
-  if (!props.isFullPage) {
-    return
-  }
-  // We only care about the case where the modal was just closed (i.e. has gone from visible -> not visible).
-  if (newV || !oldV) {
+const maybeGoBack = async () => {
+  if (!props.routeBackOnClose) {
     return
   }
+
   if (window.history.length > 1) {
     await clearError().then(router.back)
   } else {
     await clearError({ redirect: '/' })
   }
-})
+}
 
 const fullError = computed(() => {
   const err = error.value ?? modalError.value
@@ -49,6 +44,7 @@ const fullError = computed(() => {
     v-model:visible="errorModalVisible"
     header="An error ocurred"
     sub-header="Sorry about that, our team take bug reports seriously, and will try to make it right!"
+    @closed="maybeGoBack"
   >
     <StandardDebug
       label="Error Details"

frontend/composables/newModal.ts

@@ -35,7 +35,7 @@ export const useModal = () => {
     return () => loadingSet.value.delete(loadKey)
   }
   const clearLoading = () => { loadingSet.value.clear() }
-  const withLoadingAndErrorHandling = async <T> (fn: () => Promise<T>, opKey: string): (Promise<T>) => {
+  const withLoading = async <T> (fn: () => Promise<T>, opKey: string): (Promise<T>) => {
     startLoading(opKey)
     const p = fn()
     void p.finally(stopLoading(opKey))
@@ -77,7 +77,7 @@ export const useModal = () => {
     anyBlockingModalOpen,
     newModalVisibilityState,
     loading: {
-      withLoadingAndErrorHandling,
+      withLoading,
       onMountedWithLoading,
       startLoading,
       stopLoading,
@@ -88,7 +88,6 @@ export const useModal = () => {
     error: {
       error,
       errorModalVisible,
-      withLoadingAndErrorHandling,
       handleOAPIError
     },
     permissionDenied: {

frontend/composables/useAPI.ts

@@ -19,9 +19,7 @@ export const useAPI = (): API => {
   // auth.
   let headers: Record<string, string> = {}
   if (process.server) {
-    headers = Object.entries(useRequestHeaders(['cookie']))
-      .filter((ent) => !!ent[1])
-      .reduce((a, v) => ({ ...a, [v[0]]: v[1] }), {})
+    headers = useRequestHeaders(['cookie'])
   }
 
   const userCfg = {

frontend/error.vue

@@ -9,5 +9,5 @@ errorModalVisible.value = true
 </script>
 
 <template>
-  <ModalError :is-full-page="true" />
+  <ModalError :route-back-on-close="true" />
 </template>

frontend/globalimports/present.ts

@@ -1,15 +1,23 @@
-import { ErrorWithRemediation, Remediation } from '@/lib/error'
+import { createErrorWithRemediation, Remediation } from '@/lib/error'
 
-export const present = <T>(t: T | undefined | null, r: Remediation): T => {
+export const present = <T>(t: T | undefined | null, r: Remediation, cause?: string): T => {
   if (t === undefined) {
-    throw new ErrorWithRemediation(`expected to be present but was undefined: ${typeof t}.`, r)
+    throw createErrorWithRemediation({
+      name: 'present error',
+      message: 'expected to be present but was undefined',
+      cause
+    }, r)
   }
   if (t === null) {
-    throw new ErrorWithRemediation(`expected to be present but was null: ${typeof t}.`, r)
+    throw createErrorWithRemediation({
+      name: 'present error',
+      message: 'expected to be present but was null',
+      cause
+    }, r)
   }
   return t
 }
 
-export const presentOrSuggestReload = <T>(t: T | undefined | null): T => present(t, Remediation.Reload)
-export const presentOrFileBug = <T>(t: T | undefined | null): T => present(t, Remediation.FileBug)
-export const presentOrCheckURL = <T>(t: T | undefined | null): T => present(t, Remediation.CheckUrl)
+export const presentOrSuggestReload = <T>(t: T | undefined | null, cause?: string): T => present(t, Remediation.Reload, cause)
+export const presentOrFileBug = <T>(t: T | undefined | null, cause?: string): T => present(t, Remediation.FileBug, cause)
+export const presentOrCheckURL = <T>(t: T | undefined | null, cause?: string): T => present(t, Remediation.CheckUrl, cause)

frontend/layouts/default.vue

@@ -1,6 +1,7 @@
 <script setup lang="ts">
+const { loading: { onMountedWithLoading }, anyBlockingModalOpen } = useModal()
 
-const { anyBlockingModalOpen } = useModal()
+onMountedWithLoading(() => { /* nothing to do */ }, 'defaultLayout.onMountedWithLoading')
 </script>
 
 <template>

frontend/lib/error/index.ts

@@ -1,12 +1,32 @@
+import { type NuxtError } from 'nuxt/app'
+
 export enum Remediation {
   None = 'none',
   Reload = 'reload',
   FileBug = 'file-bug',
   CheckUrl = 'check-url',
 }
 
-export class ErrorWithRemediation extends Error {
-  constructor (msg: string, public readonly remediation: Remediation) {
-    super(msg)
+interface RemediationData {
+  type: 'remediation'
+  remediation: Remediation
+}
+
+type PACTAErrorData = RemediationData
+
+interface PACTAError extends NuxtError {
+  data: PACTAErrorData
+}
+
+export const createErrorWithRemediation = (err: string | Partial<NuxtError>, r: Remediation): PACTAError => {
+  const nuxtErr = createError(err)
+  if (!nuxtErr.data || typeof (nuxtErr.data) !== 'object') {
+    nuxtErr.data = {}
   }
+  nuxtErr.data.type = 'remediation'
+  nuxtErr.data.remediation = r
+
+  // TypeScript doesn't automatically pick up that `data` is always set, so we
+  // give it a nudge.
+  return nuxtErr as PACTAError
 }

frontend/pages/admin/pacta-version/[id].vue

@@ -3,31 +3,27 @@ import { type PactaVersion, type PactaVersionChanges } from '@/openapi/generated
 
 const router = useRouter()
 const { pactaClient } = useAPI()
-const { error: { withLoadingAndErrorHandling, handleOAPIError } } = useModal()
+const { loading: { withLoading }, error: { handleOAPIError } } = useModal()
 const { fromParams } = useURLParams()
 
 const id = presentOrCheckURL(fromParams('id'))
 
 const prefix = `admin/pacta-version/${id}`
 const pactaVersion = useState<PactaVersion>(`${prefix}.pactaVersion`)
 const { data: persistedPactaVersion, error, refresh } = await useAsyncData(`${prefix}.getPactaVersion`, () => {
-  return withLoadingAndErrorHandling(() => {
+  return withLoading(() => {
     return pactaClient.findPactaVersionById(id)
       .then(handleOAPIError)
   }, `${prefix}.getPactaVersion`)
 })
 if (error.value) {
   throw createError(error.value)
 }
-if (!persistedPactaVersion.value) {
-  throw createError({ message: 'PACTA version not found' })
-}
-pactaVersion.value = { ...persistedPactaVersion.value }
+
+pactaVersion.value = { ...presentOrCheckURL(persistedPactaVersion.value, 'no PACTA version in response') }
 const refreshPACTA = async () => {
   await refresh()
-  if (persistedPactaVersion.value) {
-    pactaVersion.value = { ...persistedPactaVersion.value }
-  }
+  pactaVersion.value = { ...presentOrCheckURL(persistedPactaVersion.value, 'no PACTA version in response after refresh') }
 }
 
 const changes = computed<PactaVersionChanges>(() => {
@@ -42,19 +38,19 @@ const changes = computed<PactaVersionChanges>(() => {
 })
 const hasChanges = computed<boolean>(() => Object.keys(changes.value).length > 0)
 
-const markDefault = () => withLoadingAndErrorHandling(
+const markDefault = () => withLoading(
   () => pactaClient.markPactaVersionAsDefault(id)
     .then(handleOAPIError)
     .then(refreshPACTA),
   `${prefix}.markPactaVersionAsDefault`
 )
-const deletePV = () => withLoadingAndErrorHandling(
+const deletePV = () => withLoading(
   () => pactaClient.deletePactaVersion(id)
     .then(handleOAPIError)
     .then(() => router.push('/admin/pacta-version')),
   `${prefix}.deletePactaVersion`
 )
-const saveChanges = () => withLoadingAndErrorHandling(
+const saveChanges = () => withLoading(
   () => pactaClient.updatePactaVersion(id, changes.value)
     .then(handleOAPIError)
     .then(refreshPACTA)

frontend/pages/admin/pacta-version/index.vue

@@ -1,23 +1,23 @@
 <script setup lang="ts">
 const router = useRouter()
 const { pactaClient } = useAPI()
-const { error: { withLoadingAndErrorHandling, handleOAPIError } } = useModal()
+const { loading: { withLoading }, error: { handleOAPIError } } = useModal()
 
 const prefix = 'admin/pacta-version'
 const { data: pactaVersions, refresh } = await useAsyncData(`${prefix}.getPactaVersions`, () => {
-  return withLoadingAndErrorHandling(() => {
+  return withLoading(() => {
     return pactaClient.listPactaVersions().then(handleOAPIError)
   }, `${prefix}.getPactaVersions`)
 })
 
 const newPV = () => router.push('/admin/pacta-version/new')
-const markDefault = (id: string) => withLoadingAndErrorHandling(
+const markDefault = (id: string) => withLoading(
   () => pactaClient.markPactaVersionAsDefault(id)
     .then(handleOAPIError)
     .then(refresh),
   `${prefix}.markPactaVersionAsDefault`
 )
-const deletePV = (id: string) => withLoadingAndErrorHandling(
+const deletePV = (id: string) => withLoading(
   () => pactaClient.deletePactaVersion(id)
     .then(handleOAPIError)
     .then(refresh),

frontend/pages/admin/pacta-version/new.vue

@@ -3,7 +3,7 @@ import { type PactaVersion } from '@/openapi/generated/pacta'
 
 const router = useRouter()
 const { pactaClient } = useAPI()
-const { error: { withLoadingAndErrorHandling, handleOAPIError } } = useModal()
+const { loading: { withLoading }, error: { handleOAPIError } } = useModal()
 
 const prefix = 'admin/pacta-version/new'
 const pactaVersion = useState<PactaVersion>(`${prefix}.pactaVersion`, () => ({
@@ -16,7 +16,7 @@ const pactaVersion = useState<PactaVersion>(`${prefix}.pactaVersion`, () => ({
 }))
 
 const discard = () => router.push('/admin/pacta-version')
-const save = () => withLoadingAndErrorHandling(
+const save = () => withLoading(
   () => pactaClient.createPactaVersion(pactaVersion.value).then(handleOAPIError).then(() => router.push('/admin/pacta-version')),
   `${prefix}.save`
 )