Show timeout errors on the frontend #2838
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openverse-bot
added
🟧 priority: high
obulat
commented
Aug 16, 2023
| @@ -117,7 +117,7 @@ export default defineComponent({ | |||
| }, | |||
| hasNoResults: { | |||
| type: Boolean, | |||
| required: true, | |||
| default: true, | |||
There was a problem hiding this comment.
This prop will be removed in #2811, so I set a default value for it to avoid computing it.
obulat
commented
Aug 16, 2023
| @@ -14,9 +14,6 @@ | |||
| :related-to="relatedTo" | |||
| /> | |||
| </ol> | |||
There was a problem hiding this comment.
When there is an error, VImageGrid is not displayed at all, so this was unused.
obulat
commented
Aug 16, 2023
| @@ -64,7 +61,9 @@ export default defineComponent({ | |||
| required: true, | |||
| }, | |||
| fetchState: { | |||
| type: Object as PropType<FetchState<NuxtError>>, | |||
| type: Object as PropType< | |||
There was a problem hiding this comment.
VImageGrid is used for search results that are updated in this PR, but also for the related images, which will be updated later, so we use both FetchState types here.
|
Size Change: +35.5 kB (+4%) Total Size: 927 kB
ℹ️ View Unchanged
|
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
445f129
to
702e90a
Compare
github-actions
bot
added
the
🧱 stack: documentation
|
Full-stack documentation: https://docs.openverse.org/_preview/2838 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
obulat
force-pushed
the
add/timeout-error-on-search
branch
3 times, most recently
from
14e3ff4
to
05b75f0
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
05b75f0
to
1018fca
Compare
obulat
force-pushed
the
refactor-error-page
branch
2 times, most recently
from
fcada3e
to
35b44f0
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
2 times, most recently
from
d64fcc1
to
4fcc5bf
Compare
obulat
force-pushed
the
refactor-error-page
branch
from
35b44f0
to
7be8641
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
4fcc5bf
to
ff10773
Compare
obulat
force-pushed
the
refactor-error-page
branch
2 times, most recently
from
3016353
to
632c439
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
ff10773
to
dd97dc2
Compare
obulat
force-pushed
the
refactor-error-page
branch
from
632c439
to
0ac4e07
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
2 times, most recently
from
5032b2f
to
37b4588
Compare
obulat
force-pushed
the
refactor-error-page
branch
from
5d42e71
to
3bfec52
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
e807427
to
42d2b45
Compare
dhruvkb
approved these changes
Sep 18, 2023
frontend/src/types/fetch-state.ts
Outdated
Comment on lines
10
to
12
| /** | ||
| * The error codes Axios uses. | ||
| */ |
| return props.fetchingError?.message?.includes(NO_RESULT) | ||
| ? NO_RESULT | ||
| : SERVER_TIMEOUT | ||
| return props.fetchingError.code === NO_RESULT ? NO_RESULT : SERVER_TIMEOUT |
There was a problem hiding this comment.
Nit: Can be simplified into an arrow function expression.
obulat
commented
Sep 18, 2023
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
1753943
to
e7aa46c
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
4 times, most recently
from
89d4790
to
c54f270
Compare
|
Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR: @krysal Excluding weekend1 days, this PR was ready for review 7 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)2. @obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
AetherUnbound
approved these changes
Sep 22, 2023
Comment on lines
+20
to
+22
| * The error codes Axios uses. | ||
| * @see https://github.com/axios/axios/blob/9588fcdec8aca45c3ba2f7968988a5d03f23168c/lib/core/AxiosError.js#L57C2-L71 | ||
| */ |
There was a problem hiding this comment.
Is there any chance this list could change? Is there any way to reference it directly in Axios? 😮
There was a problem hiding this comment.
This could change, but there is no way of importing the list of error kinds directly from Axios :(
Comment on lines
+136
to
150
| mediaStore.fetchState.hasStarted && | ||
| fetchingError.value !== null && | ||
| !isRetriable(fetchingError.value) | ||
| if (shouldNotRefetch) return | ||
|
|
||
| const results = await mediaStore.fetchMedia(payload) | ||
| if (!results) { | ||
| const error = mediaStore.fetchState.fetchingError | ||
| /** | ||
| * NO_RESULT error is handled by the VErrorSection component in the child pages. | ||
| * For all other errors, show the Nuxt error page. | ||
| */ | ||
| if ( | ||
| error !== null && | ||
| !error?.message?.toLowerCase().includes(NO_RESULT) && | ||
| !error?.message?.toLowerCase().includes("timeout") | ||
| ) { | ||
| return nuxtError(error) | ||
| } | ||
| await mediaStore.fetchMedia(payload) | ||
|
|
||
| if ( | ||
| fetchingError.value === null || | ||
| handledClientSide(fetchingError.value) | ||
| ) { | ||
| return null | ||
| } | ||
| return null | ||
| return nuxtError(fetchingError.value) | ||
| } |
There was a problem hiding this comment.
This feels much more robust!
frontend/src/utils/errors.ts
Outdated
| export const isRetriable = (error: FetchingError) => { | ||
| const { statusCode, code } = error | ||
| return !( | ||
| (statusCode && [429, 404, 500].includes(statusCode)) || |
There was a problem hiding this comment.
We perform very similar logic on L59 above, does it make sense to group these two at all?
obulat
force-pushed
the
add/timeout-error-on-search
branch
2 times, most recently
from
e7a99c2
to
d500d4e
Compare
obulat
force-pushed
the
add/timeout-error-on-search
branch
from
bd30ccc
to
fdfe1f5
Compare
ngken0995
pushed a commit
to ngken0995/openverse
that referenced
this pull request
Oct 3, 2023
* Show timeout errors on the frontend * Use FetchingError in all stores * Fix error * Show client-side errors on single result pages * Set 500 as a non-retriable error * Add changes from code review * Use local base64 image for thumbnail * Fix footer * Fix image-cell test * Extract common error checking functionality * Update unit tests
ngken0995
added a commit
to ngken0995/openverse
that referenced
this pull request
Oct 4, 2023
…WordPress#3055) * change deprecated ES search body * trim white space Add thumbnail repsonse time runbooks (WordPress#3053) * Add thumb repsonse time runbooks * Add files to index * Threshold alarms are low severity if not anomalous generate-dag-docs recipe move DAGs.md to documentation folder (WordPress#3061) Co-authored-by: Madison Swain-Bowden <[email protected]> Upgrade psycopg to version 3 in the API (WordPress#3064) Update VSourcesTable.vue (WordPress#3026) Co-authored-by: sarayourfriend <[email protected]> Co-authored-by: Olga Bulat <[email protected]> Co-authored-by: Krystle Salazar <[email protected]> Transfer UUID validation inside serializer (WordPress#3068) * Transfer UUID validation inside serializer * Add test case Publish changelog for api-2023.09.28.00.26.34 (WordPress#3070) Co-authored-by: AetherUnbound <[email protected]> Use fully qualified docker image names (WordPress#3071) Publish changelog for ingestion_server-2023.09.29.17.40.50 (WordPress#3082) Co-authored-by: stacimc <[email protected]> Update `_AIRFLOW_DB_UPGRADE` to `_AIRFLOW_DB_MIGRATE` Increase the API sources cache TTL from 20 minutes to 4 hours (WordPress#3083) Publish changelog for api-2023.09.30.00.15.32 (WordPress#3084) Co-authored-by: AetherUnbound <[email protected]> Bump ipython from 8.14.0 to 8.16.0 in /automations/python (WordPress#3099) Bumps [ipython](https://github.com/ipython/ipython) from 8.14.0 to 8.16.0. - [Release notes](https://github.com/ipython/ipython/releases) - [Commits](ipython/ipython@8.14.0...8.16.0) --- updated-dependencies: - dependency-name: ipython dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Remove boto3 dependency (WordPress#3073) Bump docker/login-action from 2 to 3 (WordPress#3089) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump elasticsearch from 8.8.2 to 8.10.0 in /api (WordPress#3103) Bumps [elasticsearch](https://github.com/elastic/elasticsearch-py) from 8.8.2 to 8.10.0. - [Release notes](https://github.com/elastic/elasticsearch-py/releases) - [Commits](elastic/elasticsearch-py@v8.8.2...v8.10.0) --- updated-dependencies: - dependency-name: elasticsearch dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump psycopg2 from 2.9.7 to 2.9.8 in /ingestion_server (WordPress#3097) Bumps [psycopg2](https://github.com/psycopg/psycopg2) from 2.9.7 to 2.9.8. - [Changelog](https://github.com/psycopg/psycopg2/blob/master/NEWS) - [Commits](psycopg/psycopg2@2.9.7...2.9.8) --- updated-dependencies: - dependency-name: psycopg2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump elasticsearch from 8.8.2 to 8.10.0 in /ingestion_server (WordPress#3095) Bumps [elasticsearch](https://github.com/elastic/elasticsearch-py) from 8.8.2 to 8.10.0. - [Release notes](https://github.com/elastic/elasticsearch-py/releases) - [Commits](elastic/elasticsearch-py@v8.8.2...v8.10.0) --- updated-dependencies: - dependency-name: elasticsearch dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump pygithub from 1.59.1 to 2.1.1 in /automations/python (WordPress#3102) Bumps [pygithub](https://github.com/pygithub/pygithub) from 1.59.1 to 2.1.1. - [Release notes](https://github.com/pygithub/pygithub/releases) - [Changelog](https://github.com/PyGithub/PyGithub/blob/main/doc/changes.rst) - [Commits](PyGithub/PyGithub@v1.59.1...v2.1.1) --- updated-dependencies: - dependency-name: pygithub dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump furo from 2023.8.19 to 2023.9.10 in /documentation (WordPress#3092) Bumps [furo](https://github.com/pradyunsg/furo) from 2023.8.19 to 2023.9.10. - [Release notes](https://github.com/pradyunsg/furo/releases) - [Changelog](https://github.com/pradyunsg/furo/blob/main/docs/changelog.md) - [Commits](pradyunsg/furo@2023.08.19...2023.09.10) --- updated-dependencies: - dependency-name: furo dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump docker/build-push-action from 4 to 5 (WordPress#3090) Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 4 to 5. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](docker/build-push-action@v4...v5) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump actions/checkout from 3 to 4 (WordPress#3088) Bump docker/setup-buildx-action from 2 to 3 (WordPress#3087) Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2 to 3. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v2...v3) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump fakeredis from 2.18.0 to 2.19.0 in /api (WordPress#3100) Bumps [fakeredis](https://github.com/cunla/fakeredis-py) from 2.18.0 to 2.19.0. - [Release notes](https://github.com/cunla/fakeredis-py/releases) - [Commits](cunla/fakeredis-py@v2.18.0...v2.19.0) --- updated-dependencies: - dependency-name: fakeredis dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Show timeout errors on the frontend (WordPress#2838) * Show timeout errors on the frontend * Use FetchingError in all stores * Fix error * Show client-side errors on single result pages * Set 500 as a non-retriable error * Add changes from code review * Use local base64 image for thumbnail * Fix footer * Fix image-cell test * Extract common error checking functionality * Update unit tests Add runbooks for API Thumbnails 2XX/5XX responses and Request Count alarms (WordPress#3076) Bump jsonschema from 4.19.0 to 4.19.1 in /ingestion_server (WordPress#3098) Bumps [jsonschema](https://github.com/python-jsonschema/jsonschema) from 4.19.0 to 4.19.1. - [Release notes](https://github.com/python-jsonschema/jsonschema/releases) - [Changelog](https://github.com/python-jsonschema/jsonschema/blob/main/CHANGELOG.rst) - [Commits](python-jsonschema/jsonschema@v4.19.0...v4.19.1) --- updated-dependencies: - dependency-name: jsonschema dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump ipython from 8.15.0 to 8.16.1 in /ingestion_server (WordPress#3110) Bumps [ipython](https://github.com/ipython/ipython) from 8.15.0 to 8.16.1. - [Release notes](https://github.com/ipython/ipython/releases) - [Commits](https://github.com/ipython/ipython/commits) --- updated-dependencies: - dependency-name: ipython dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump sphinx from 7.2.5 to 7.2.6 in /documentation (WordPress#3091) Bumps [sphinx](https://github.com/sphinx-doc/sphinx) from 7.2.5 to 7.2.6. - [Release notes](https://github.com/sphinx-doc/sphinx/releases) - [Changelog](https://github.com/sphinx-doc/sphinx/blob/master/CHANGES.rst) - [Commits](sphinx-doc/sphinx@v7.2.5...v7.2.6) --- updated-dependencies: - dependency-name: sphinx dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump tldextract from 3.4.4 to 3.6.0 in /catalog (WordPress#3093) Bumps [tldextract](https://github.com/john-kurkowski/tldextract) from 3.4.4 to 3.6.0. - [Release notes](https://github.com/john-kurkowski/tldextract/releases) - [Changelog](https://github.com/john-kurkowski/tldextract/blob/master/CHANGELOG.md) - [Commits](john-kurkowski/tldextract@3.4.4...3.6.0) --- updated-dependencies: - dependency-name: tldextract dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump urllib3 from 2.0.5 to 2.0.6 in /documentation (WordPress#3120) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump urllib3 from 1.26.16 to 1.26.17 in /api (WordPress#3123) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump urllib3 from 2.0.5 to 2.0.6 in /automations/python (WordPress#3119) Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.0.5 to 2.0.6. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](urllib3/urllib3@v2.0.5...2.0.6) --- updated-dependencies: - dependency-name: urllib3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Bump urllib3 from 1.26.16 to 1.26.17 in /ingestion_server (WordPress#3124) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> implement vale determine files mircosoft sentence format change alert level change alert level change alert level change alert level change alert level change alert level change alert level Empty-Commit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #632 by @sarayourfriend
Description
This PR updates the
FetchStateto always use theFetchingErrorinstead ofNuxtError|string|FetchingErrorfor the error object.FetchingErrorinterface provides more information about the error, including the response status code, the string code of the error from axios, the kind of request (single result, search, provider or the related media), and additional details such as the item id.This interface makes it easier to show the timeout error pages on
SERVER_TIMEOUTorECONNABORTEDerrors.FetchStatewas made a generic to allow the use ofstringandNuxtErrorfor thefetchingErrorvalue. This PR removes this to make fetchState simpler.Testing Instructions
To simulate the server timeout, lower the default timeout to 30:
openverse/frontend/src/data/api-service.ts
Line 8 in 026a328
Now, you should see the timeout error page any time you search for something. You should also see the same page if you refresh this search page (so, the page will be server-rendered now).
To see the no results error page, search for something like "querywithnoresults". It should work for all search types, both on CSR and SSR, and should not try to reload the page on SSR.
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin