From 6d96f0b0d1c74523b9f3d26360aa63d92c307959 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 11 Sep 2023 10:25:10 +0200 Subject: [PATCH] Add fetch wrappers, ignore network errors in actions view (#26985) 1. Introduce lightweight `fetch` wrapper functions that automatically sets csfr token, content-type and use it in `RepoActionView.vue`. 2. Fix a specific issue on `RepoActionView.vue` where a fetch network error is shortly visible during page reload sometimes. It can be reproduced by F5-in in quick succession on the actions view page and was also producing a red error box on the page. Once approved, we can replace all current `fetch` uses in UI with this in another PR. --------- Co-authored-by: Giteabot --- .../contributing/guidelines-frontend.en-us.md | 6 +++ web_src/js/components/RepoActionView.vue | 52 +++++++++---------- web_src/js/modules/fetch.js | 38 ++++++++++++++ web_src/js/modules/fetch.test.js | 11 ++++ 4 files changed, 79 insertions(+), 28 deletions(-) create mode 100644 web_src/js/modules/fetch.js create mode 100644 web_src/js/modules/fetch.test.js diff --git a/docs/content/contributing/guidelines-frontend.en-us.md b/docs/content/contributing/guidelines-frontend.en-us.md index dc9eef1303..921c2b0233 100644 --- a/docs/content/contributing/guidelines-frontend.en-us.md +++ b/docs/content/contributing/guidelines-frontend.en-us.md @@ -92,6 +92,12 @@ it's recommended to use `const _promise = asyncFoo()` to tell readers that this is done by purpose, we want to call the async function and ignore the Promise. Some lint rules and IDEs also have warnings if the returned Promise is not handled. +### Fetching data + +To fetch data, use the wrapper functions `GET`, `POST` etc. from `modules/fetch.js`. They +accept a `data` option for the content, will automatically set CSFR token and return a +Promise for a [Response](https://developer.mozilla.org/en-US/docs/Web/API/Response). + ### HTML Attributes and `dataset` The usage of `dataset` is forbidden, its camel-casing behaviour makes it hard to grep for attributes. diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 925ff7e087..da06dd9cb6 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -5,8 +5,7 @@ import {createApp} from 'vue'; import {toggleElem} from '../utils/dom.js'; import {getCurrentLocale} from '../utils.js'; import {renderAnsi} from '../render/ansi.js'; - -const {csrfToken} = window.config; +import {POST} from '../modules/fetch.js'; const sfc = { name: 'RepoActionView', @@ -145,11 +144,11 @@ const sfc = { }, // cancel a run cancelRun() { - this.fetchPost(`${this.run.link}/cancel`); + POST(`${this.run.link}/cancel`); }, // approve a run approveRun() { - this.fetchPost(`${this.run.link}/approve`); + POST(`${this.run.link}/approve`); }, createLogLine(line, startTime, stepIndex) { @@ -196,6 +195,11 @@ const sfc = { } }, + async fetchArtifacts() { + const resp = await POST(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); + return await resp.json(); + }, + async fetchJob() { const logCursors = this.currentJobStepsStates.map((it, idx) => { // cursor is used to indicate the last position of the logs @@ -203,10 +207,9 @@ const sfc = { // for example: make cursor=null means the first time to fetch logs, cursor=eof means no more logs, etc return {step: idx, cursor: it.cursor, expanded: it.expanded}; }); - const resp = await this.fetchPost( - `${this.actionsURL}/runs/${this.runIndex}/jobs/${this.jobIndex}`, - JSON.stringify({logCursors}), - ); + const resp = await POST(`${this.actionsURL}/runs/${this.runIndex}/jobs/${this.jobIndex}`, { + data: {logCursors}, + }); return await resp.json(); }, @@ -215,16 +218,21 @@ const sfc = { try { this.loading = true; - // refresh artifacts if upload-artifact step done - const resp = await this.fetchPost(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); - const artifacts = await resp.json(); + let job, artifacts; + try { + [job, artifacts] = await Promise.all([ + this.fetchJob(), + this.fetchArtifacts(), // refresh artifacts if upload-artifact step done + ]); + } catch (err) { + if (!(err instanceof TypeError)) throw err; // avoid network error while unloading page + } + this.artifacts = artifacts['artifacts'] || []; - const response = await this.fetchJob(); - // save the state to Vue data, then the UI will be updated - this.run = response.state.run; - this.currentJob = response.state.currentJob; + this.run = job.state.run; + this.currentJob = job.state.currentJob; // sync the currentJobStepsStates to store the job step states for (let i = 0; i < this.currentJob.steps.length; i++) { @@ -234,7 +242,7 @@ const sfc = { } } // append logs to the UI - for (const logs of response.logs.stepsLog) { + for (const logs of job.logs.stepsLog) { // save the cursor, it will be passed to backend next time this.currentJobStepsStates[logs.step].cursor = logs.cursor; this.appendLogs(logs.step, logs.lines, logs.started); @@ -249,18 +257,6 @@ const sfc = { } }, - - fetchPost(url, body) { - return fetch(url, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Csrf-Token': csrfToken, - }, - body, - }); - }, - isDone(status) { return ['success', 'skipped', 'failure', 'cancelled'].includes(status); }, diff --git a/web_src/js/modules/fetch.js b/web_src/js/modules/fetch.js new file mode 100644 index 0000000000..63732206c6 --- /dev/null +++ b/web_src/js/modules/fetch.js @@ -0,0 +1,38 @@ +import {isObject} from '../utils.js'; + +const {csrfToken} = window.config; + +// fetch wrapper, use below method name functions and the `data` option to pass in data +// which will automatically set an appropriate content-type header. For json content, +// only object and array types are currently supported. +function request(url, {headers, data, body, ...other} = {}) { + let contentType; + if (!body) { + if (data instanceof FormData) { + contentType = 'multipart/form-data'; + body = data; + } else if (data instanceof URLSearchParams) { + contentType = 'application/x-www-form-urlencoded'; + body = data; + } else if (isObject(data) || Array.isArray(data)) { + contentType = 'application/json'; + body = JSON.stringify(data); + } + } + + return fetch(url, { + headers: { + 'x-csrf-token': csrfToken, + ...(contentType && {'content-type': contentType}), + ...headers, + }, + ...(body && {body}), + ...other, + }); +} + +export const GET = (url, opts) => request(url, {method: 'GET', ...opts}); +export const POST = (url, opts) => request(url, {method: 'POST', ...opts}); +export const PATCH = (url, opts) => request(url, {method: 'PATCH', ...opts}); +export const PUT = (url, opts) => request(url, {method: 'PUT', ...opts}); +export const DELETE = (url, opts) => request(url, {method: 'DELETE', ...opts}); diff --git a/web_src/js/modules/fetch.test.js b/web_src/js/modules/fetch.test.js new file mode 100644 index 0000000000..ec0377b4d9 --- /dev/null +++ b/web_src/js/modules/fetch.test.js @@ -0,0 +1,11 @@ +import {test, expect} from 'vitest'; +import {GET, POST, PATCH, PUT, DELETE} from './fetch.js'; + +// tests here are only to satisfy the linter for unused functions +test('exports', () => { + expect(GET).toBeTruthy(); + expect(POST).toBeTruthy(); + expect(PATCH).toBeTruthy(); + expect(PUT).toBeTruthy(); + expect(DELETE).toBeTruthy(); +});