Skip to content
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

Show timeout errors on the frontend #2838

Merged
merged 11 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions frontend/src/components/VErrorSection/VErrorSection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
<script lang="ts">
import { computed, defineComponent, PropType } from "vue"

import { NO_RESULT, SERVER_TIMEOUT } from "~/constants/errors"
import { useSearchStore } from "~/stores/search"
import { ECONNABORTED, NO_RESULT, SERVER_TIMEOUT } from "~/constants/errors"

import type { NuxtError } from "@nuxt/types"
import type { FetchingError } from "~/types/fetch-state"

export default defineComponent({
components: {
Expand All @@ -30,26 +29,25 @@ export default defineComponent({
},
props: {
fetchingError: {
type: Object as PropType<NuxtError>,
type: Object as PropType<FetchingError>,
required: true,
},
},
setup(props) {
const searchStore = useSearchStore()
const searchTerm = computed(() => searchStore.searchTerm)
const searchTerm = computed(
() => props.fetchingError.details?.searchTerm ?? ""
)
/**
* The code used for the error page image.
* For now, NO_RESULT image is used for searches without result,
* and SERVER_TIMEOUT image is used as a fall-back for all other errors.
*/
const errorCode = computed(() => {
return props.fetchingError?.message?.includes(NO_RESULT)
? NO_RESULT
: SERVER_TIMEOUT
})
const errorCode = computed(() =>
props.fetchingError.code === NO_RESULT ? NO_RESULT : SERVER_TIMEOUT
)

const isTimeout = computed(() =>
props.fetchingError?.message?.toLowerCase().includes("timeout")
[SERVER_TIMEOUT, ECONNABORTED].includes(props.fetchingError.code)
)

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {

import VErrorSection from "~/components/VErrorSection/VErrorSection.vue"

import { NO_RESULT } from "~/constants/errors"
import { useSearchStore } from "@/stores/search"
import { ECONNABORTED, ERR_UNKNOWN, NO_RESULT } from "~/constants/errors"

<Meta title="Components/VErrorSection" component={VErrorSection} />

Expand All @@ -27,10 +26,6 @@ export const ErrorPageTemplate = (args) => ({
template: `<VErrorSection v-bind="args" />`,
components: { VErrorSection },
setup() {
const searchStore = useSearchStore()
searchStore.setSearchTerm(
args.fetchingError.details?.searchTerm ?? "sad person"
)
return { args }
},
})
Expand All @@ -40,7 +35,7 @@ export const ErrorPageTemplate = (args) => ({
name="No result"
args={{
fetchingError: {
message: NO_RESULT,
code: NO_RESULT,
requestKind: "search",
searchType: "image",
details: { searchTerm: "sad person" },
Expand All @@ -60,7 +55,7 @@ This result appears when an API request times out.
name="Server timeout"
args={{
fetchingError: {
message: "server timeout",
code: ECONNABORTED,
requestKind: "search",
searchType: "image",
},
Expand All @@ -74,15 +69,17 @@ This result appears when an API request times out.

This page is shown when there is some other error.

<Story
name="Unknown error"
args={{
fetchingError: {
message: "Unknown error",
requestKind: "search",
searchType: "image",
},
}}
>
{ErrorPageTemplate.bind({})}
</Story>
<Canvas>
<Story
name="Unknown error"
args={{
fetchingError: {
code: ERR_UNKNOWN,
requestKind: "search",
searchType: "image",
},
}}
>
{ErrorPageTemplate.bind({})}
</Story>
</Canvas>
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export default defineComponent({
},
hasNoResults: {
type: Boolean,
required: true,
default: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This prop will be removed in #2811, so I set a default value for it to avoid computing it.

},
},
emits: {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VFooter/VFooter.vue
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export default defineComponent({
/*
We set the number of rows in JS to have 2 equally distributed link columns.
*/
grid-template-rows: repeat(var(--link-col-height, 3), auto);
grid-template-rows: repeat(var(--link-col-height, 4), auto);
}

.footer-lg .nav-list {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ import VLoadMore from "~/components/VLoadMore.vue"
import VGridSkeleton from "~/components/VSkeleton/VGridSkeleton.vue"
import VSnackbar from "~/components/VSnackbar.vue"

import type { NuxtError } from "@nuxt/types"

/**
* This component shows a loading skeleton if the results are not yet loaded,
* and then shows the list of audio, with the Load more button if needed.
Expand All @@ -66,7 +64,7 @@ export default defineComponent({
required: true,
},
fetchState: {
type: Object as PropType<FetchState<NuxtError> | FetchState>,
type: Object as PropType<FetchState>,
required: true,
},
/**
Expand Down
10 changes: 2 additions & 8 deletions frontend/src/components/VSearchResultsGrid/VImageGrid.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@
:related-to="relatedTo"
/>
</ol>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When there is an error, VImageGrid is not displayed at all, so this was unused.

<h5 v-if="isError && !fetchState.isFinished" class="py-4">
{{ fetchState.fetchingError }}
</h5>
<footer v-if="!isSinglePage" class="pt-4">
<VLoadMore />
</footer>
Expand All @@ -43,8 +40,6 @@ import VGridSkeleton from "~/components/VSkeleton/VGridSkeleton.vue"
import VLoadMore from "~/components/VLoadMore.vue"
import VImageCell from "~/components/VImageCell/VImageCell.vue"

import type { NuxtError } from "@nuxt/types"

export default defineComponent({
name: "ImageGrid",
components: { VGridSkeleton, VLoadMore, VImageCell },
Expand All @@ -64,7 +59,7 @@ export default defineComponent({
required: true,
},
fetchState: {
type: Object as PropType<FetchState | FetchState<NuxtError>>,
type: Object as PropType<FetchState>,
required: true,
},
imageGridLabel: {
Expand All @@ -76,13 +71,12 @@ export default defineComponent({
const searchStore = useSearchStore()

const searchTerm = computed(() => searchStore.searchTerm)
const isError = computed(() => props.fetchState.fetchingError !== null)

const relatedTo = computed(() => {
return props.isSinglePage ? useRelatedMediaStore().mainMediaId : null
})

return { isError, searchTerm, relatedTo }
return { searchTerm, relatedTo }
},
})
</script>
Expand Down
40 changes: 39 additions & 1 deletion frontend/src/constants/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,45 @@

export const NO_RESULT = "NO_RESULT"
export const SERVER_TIMEOUT = "SERVER_TIMEOUT"
export const ECONNABORTED = "ECONNABORTED"

export const errorCodes = [NO_RESULT, SERVER_TIMEOUT] as const
export const ERR_UNKNOWN = "ERR_UNKNOWN"

export const customErrorCodes = [
NO_RESULT,
SERVER_TIMEOUT,
ECONNABORTED,
ERR_UNKNOWN,
] as const

/**
* The error codes Axios uses.
* @see https://github.com/axios/axios/blob/9588fcdec8aca45c3ba2f7968988a5d03f23168c/lib/core/AxiosError.js#L57C2-L71
*/
Comment on lines +20 to +22
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any chance this list could change? Is there any way to reference it directly in Axios? 😮

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could change, but there is no way of importing the list of error kinds directly from Axios :(

const axiosErrorCodes = [
"ERR_BAD_OPTION_VALUE",
"ERR_BAD_OPTION",
"ECONNABORTED",
"ETIMEDOUT",
"ERR_NETWORK",
"ERR_FR_TOO_MANY_REDIRECTS",
"ERR_DEPRECATED",
"ERR_BAD_RESPONSE",
"ERR_BAD_REQUEST",
"ERR_CANCELED",
"ERR_NOT_SUPPORT",
"ERR_INVALID_URL",
] as const

export const errorCodes = [...customErrorCodes, ...axiosErrorCodes] as const

export const clientSideErrorCodes: readonly ErrorCode[] = [
ECONNABORTED,
SERVER_TIMEOUT,
NO_RESULT,
ERR_UNKNOWN,
"ERR_NETWORK",
"ETIMEDOUT",
] as const

export type ErrorCode = (typeof errorCodes)[number]
11 changes: 3 additions & 8 deletions frontend/src/middleware/search.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useSearchStore } from "~/stores/search"
import { useMediaStore } from "~/stores/media"
import { NO_RESULT } from "~/constants/errors"

import { handledClientSide } from "~/utils/errors"

import type { Middleware } from "@nuxt/types"

Expand Down Expand Up @@ -44,13 +45,7 @@ export const searchMiddleware: Middleware = async ({
const results = await mediaStore.fetchMedia()

const fetchingError = mediaStore.fetchState.fetchingError
// NO_RESULTS and timeout are handled client-side, for other errors show server error page
if (
!results &&
fetchingError &&
!fetchingError?.message?.includes(NO_RESULT) &&
!fetchingError?.message?.includes("timeout")
) {
if (!results && fetchingError && !handledClientSide(fetchingError)) {
nuxtError(fetchingError)
}
}
Expand Down
9 changes: 7 additions & 2 deletions frontend/src/middleware/single-result.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useSingleResultStore } from "~/stores/media/single-result"
import { useSearchStore } from "~/stores/search"
import { useRelatedMediaStore } from "~/stores/media/related-media"
import { isRetriable } from "~/utils/errors"

import { AUDIO, IMAGE } from "~/constants/media"

Expand All @@ -17,11 +18,15 @@ export const singleResultMiddleware: Middleware = async ({

if (process.server) {
const media = await singleResultStore.fetch(mediaType, route.params.id)
await useRelatedMediaStore($pinia).fetchMedia(mediaType, route.params.id)

if (!media) {
error(singleResultStore.fetchState.fetchingError ?? {})
const fetchingError = singleResultStore.fetchState.fetchingError

if (fetchingError && !isRetriable(fetchingError)) {
error(fetchingError ?? {})
}
}
await useRelatedMediaStore($pinia).fetchMedia(mediaType, route.params.id)
} else {
// Client-side rendering
singleResultStore.setMediaById(mediaType, route.params.id)
Expand Down
13 changes: 11 additions & 2 deletions frontend/src/pages/audio/_id/index.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<template>
<main :id="skipToContentTargetId" tabindex="-1" class="relative flex-grow">
<template v-if="audio">
<VErrorSection
v-if="fetchingError"
:fetching-error="fetchingError"
class="px-6 py-10 lg:px-10"
/>
<template v-else-if="audio">
<VSafetyWall v-if="isHidden" :media="audio" @reveal="reveal" />
<template v-else>
<VSingleResultControls :media="audio" />
Expand Down Expand Up @@ -33,7 +38,7 @@
</template>

<script lang="ts">
import { ref } from "vue"
import { computed, ref } from "vue"
import {
defineComponent,
useContext,
Expand Down Expand Up @@ -83,6 +88,9 @@ export default defineComponent({
const route = useRoute()

const audio = ref<AudioDetail | null>(singleResultStore.audio)
const fetchingError = computed(
() => singleResultStore.fetchState.fetchingError
)

const { error: nuxtError } = useContext()

Expand Down Expand Up @@ -121,6 +129,7 @@ export default defineComponent({

return {
audio,
fetchingError,

sendAudioEvent,

Expand Down
16 changes: 14 additions & 2 deletions frontend/src/pages/image/_id/index.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<template>
<main :id="skipToContentTargetId" tabindex="-1" class="relative flex-grow">
<template v-if="image">
<VErrorSection
v-if="fetchingError"
:fetching-error="fetchingError"
class="px-6 py-10 lg:px-10"
/>
<template v-else-if="image">
<VSafetyWall v-if="isHidden" :media="image" @reveal="reveal" />
<template v-else>
<VSingleResultControls :media="image" />
Expand Down Expand Up @@ -114,6 +119,7 @@ import { useAnalytics } from "~/composables/use-analytics"
import { useSensitiveMedia } from "~/composables/use-sensitive-media"
import { useSingleResultPageMeta } from "~/composables/use-single-result-page-meta"

import { isRetriable } from "~/utils/errors"
import { useSingleResultStore } from "~/stores/media/single-result"
import { singleResultMiddleware } from "~/middleware/single-result"

Expand Down Expand Up @@ -153,6 +159,9 @@ export default defineComponent({
const route = useRoute()

const image = ref<ImageDetail | null>(singleResultStore.image)
const fetchingError = computed(
() => singleResultStore.fetchState.fetchingError
)

/**
* To make sure that image is loaded fast, we `src` to `image.thumbnail`,
Expand All @@ -168,7 +177,9 @@ export default defineComponent({
const imageId = route.value.params.id
const fetchedImage = await singleResultStore.fetch(IMAGE, imageId)
if (!fetchedImage) {
nuxtError(singleResultStore.fetchState.fetchingError ?? {})
if (fetchingError.value && !isRetriable(fetchingError.value)) {
nuxtError(fetchingError.value)
}
} else {
image.value = fetchedImage
imageSrc.value = fetchedImage.thumbnail
Expand Down Expand Up @@ -279,6 +290,7 @@ export default defineComponent({

return {
image,
fetchingError,
imageWidth,
imageHeight,
imageSrc,
Expand Down
Loading
Loading