update resources on script update/save and display errors (#879)

* refactor: allow more props in one-line literal objects

* feat: keep message while hovered

* feat: improve showMessage with multi-line text

* feat: update resources on script update/save

* always fetch from network and rely on browser caching
  (but the installer page still uses cache for resiliency)
* force no-cache HTTP header on script update
* notify about errors for resource fetches and display them while saving in editor or updating in dashboard

* fixup! rename parameter

* fixup! swap params

* fixup! rename var

.eslintrc.js

@@ -26,5 +26,12 @@ module.exports = {
       js: 'never',
       vue: 'never',
     }],
+    // copied from airbnb-base, replaced 4 with 8
+    'object-curly-newline': ['error', {
+      ObjectExpression: { minProperties: 8, multiline: true, consistent: true },
+      ObjectPattern: { minProperties: 8, multiline: true, consistent: true },
+      ImportDeclaration: { minProperties: 8, multiline: true, consistent: true },
+      ExportDeclaration: { minProperties: 8, multiline: true, consistent: true },
+    }],
   },
 };

src/_locales/en/messages.yml

@@ -459,6 +459,9 @@ menuScriptEnabled:
 msgCheckingForUpdate:
   description: Message shown when a script is being checked for updates by version numbers.
   message: Checking for updates...
+msgErrorFetchingResource:
+  description: Message shown when Violentmonkey fails fetching a resource/require/icon of the script.
+  message: Error fetching resource!
 msgErrorFetchingScript:
   description: Message shown when Violentmonkey fails fetching a new version of the script.
   message: Error fetching script!

src/background/utils/db.js

@@ -1,18 +1,15 @@
-import {
-  i18n, getFullUrl, isRemote, getRnd4, sendCmd,
-} from '#/common';
+import { i18n, getFullUrl, isRemote, getRnd4, sendCmd, trueJoin } from '#/common';
 import { CMD_SCRIPT_ADD, CMD_SCRIPT_UPDATE, TIMEOUT_WEEK } from '#/common/consts';
 import { forEachEntry, forEachKey, forEachValue } from '#/common/object';
 import storage from '#/common/storage';
 import pluginEvents from '../plugin/events';
-import {
-  getNameURI, parseMeta, newScript, getDefaultCustom,
-} from './script';
+import { getNameURI, parseMeta, newScript, getDefaultCustom } from './script';
 import { testScript, testBlacklist } from './tester';
 import { preInitialize } from './init';
 import { commands } from './message';
 import patchDB from './patch-db';
 import { setOption } from './options';
+import './storage-fetch';
 
 const store = {};
 
@@ -102,6 +99,10 @@ preInitialize.push(async () => {
   };
   const idMap = {};
   const uriMap = {};
+  const mods = [];
+  const resUrls = [];
+  /** @this VMScriptCustom.pathMap */
+  const rememberUrl = function _(url) { resUrls.push(this[url] || url); };
   data::forEachEntry(([key, script]) => {
     if (key.startsWith(storage.script.prefix)) {
       // {
@@ -136,8 +137,19 @@ preInitialize.push(async () => {
       storeInfo.id = Math.max(storeInfo.id, id);
       storeInfo.position = Math.max(storeInfo.position, getInt(script.props.position));
       scripts.push(script);
+      // listing all known resource urls in order to remove unused mod keys
+      const {
+        custom: { pathMap = {} } = {},
+        meta = {},
+      } = script;
+      meta.require?.forEach(rememberUrl, pathMap);
+      Object.values(meta.resources || {}).forEach(rememberUrl, pathMap);
+      pathMap::rememberUrl(meta.icon);
+    } else if (key.startsWith(storage.mod.prefix)) {
+      mods.push(key.slice(storage.mod.prefix.length));
     }
   });
+  storage.mod.removeMulti(mods.filter(url => !resUrls.includes(url)));
   Object.assign(store, {
     scripts,
     storeInfo,
@@ -433,7 +445,7 @@ export async function parseScript(src) {
   if (src.position) script.props.position = +src.position;
   buildPathMap(script, src.url);
   await saveScript(script, src.code);
-  fetchScriptResources(script, src);
+  fetchResources(script, src);
   Object.assign(result.update, script, src.update);
   result.where = { id: script.props.id };
   sendCmd(cmd, result);
@@ -460,50 +472,54 @@ function buildPathMap(script, base) {
   return pathMap;
 }
 
-/** @return {void} */
-function fetchScriptResources(script, cache) {
-  const { meta, custom: { pathMap } } = script;
-  // @require
-  meta.require.forEach((key) => {
-    const fullUrl = pathMap[key] || key;
-    const cached = cache.require?.[fullUrl];
-    if (cached) {
-      storage.require.set(fullUrl, cached);
-    } else {
-      storage.require.fetch(fullUrl);
-    }
-  });
-  // @resource
-  meta.resources::forEachValue((url) => {
-    const fullUrl = pathMap[url] || url;
-    const cached = cache.resources?.[fullUrl];
-    if (cached) {
-      storage.cache.set(fullUrl, cached);
-    } else {
-      storage.cache.fetch(fullUrl);
+/** @return {Promise<?string>} resolves to error text if `resourceCache` is absent */
+export async function fetchResources(script, resourceCache, reqOptions) {
+  const { custom: { pathMap }, meta } = script;
+  const snatch = (url, type, validator) => {
+    url = pathMap[url] || url;
+    const contents = resourceCache?.[type][url];
+    return contents != null && !validator
+      ? storage[type].set(url, contents) && null
+      : storage[type].fetch(url, reqOptions, validator).catch(err => err);
+  };
+  const errors = await Promise.all([
+    ...meta.require.map(url => snatch(url, 'require')),
+    ...Object.values(meta.resources).map(url => snatch(url, 'cache')),
+    isRemote(meta.icon) && snatch(meta.icon, 'cache', validateImage),
+  ]);
+  if (!resourceCache) {
+    const error = errors.map(formatHttpError)::trueJoin('\n');
+    if (error) {
+      const message = i18n('msgErrorFetchingResource');
+      sendCmd(CMD_SCRIPT_UPDATE, {
+        update: { error, message },
+        where: { id: script.props.id },
+      });
+      return `${message}\n${error}`;
     }
-  });
-  // @icon
-  if (isRemote(meta.icon)) {
-    const fullUrl = pathMap[meta.icon] || meta.icon;
-    storage.cache.fetch(fullUrl, ({ blob: getBlob }) => new Promise((resolve, reject) => {
-      const blob = getBlob();
-      const url = URL.createObjectURL(blob);
-      const image = new Image();
-      const free = () => URL.revokeObjectURL(url);
-      image.onload = () => {
-        free();
-        resolve();
-      };
-      image.onerror = () => {
-        free();
-        reject({ type: 'IMAGE_ERROR', url });
-      };
-      image.src = url;
-    }));
   }
 }
 
+/** @return {Promise<void>} resolves on success, rejects on error */
+function validateImage(url, buf, type) {
+  return new Promise((resolve, reject) => {
+    const blobUrl = URL.createObjectURL(new Blob([buf], { type }));
+    const onDone = (e) => {
+      URL.revokeObjectURL(blobUrl);
+      if (e.type === 'load') resolve();
+      else reject({ type: 'IMAGE_ERROR', url });
+    };
+    const image = new Image();
+    image.onload = onDone;
+    image.onerror = onDone;
+    image.src = blobUrl;
+  });
+}
+
+function formatHttpError(e) {
+  return e && [e.status && `HTTP${e.status}`, e.url]::trueJoin(' ') || e;
+}
+
 /** @return {Promise<void>} */
 export async function vacuum() {
   const valueKeys = {};

src/background/utils/storage-fetch.js

@@ -0,0 +1,54 @@
+import { buffer2string, request } from '#/common';
+import storage from '#/common/storage';
+
+/** @type { function(url, options, check): Promise<void> } or throws on error */
+storage.cache.fetch = cacheOrFetch({
+  init(options) {
+    return { ...options, responseType: 'arraybuffer' };
+  },
+  async transform(response, url, options, check) {
+    const contentType = (response.xhr.getResponseHeader('content-type') || '').split(';')[0];
+    await check?.(url, response.data, contentType);
+    return `${contentType},${btoa(buffer2string(response.data))}`;
+  },
+});
+
+/** @type { function(url, options): Promise<void> } or throws on error */
+storage.require.fetch = cacheOrFetch();
+
+function cacheOrFetch(handlers = {}) {
+  const requests = {};
+  const { init, transform } = handlers;
+  /** @this storage.<area> */
+  return function cacheOrFetchHandler(...args) {
+    const [url] = args;
+    const promise = requests[url] || (requests[url] = this::doFetch(...args));
+    return promise;
+  };
+  /** @this storage.<area> */
+  async function doFetch(...args) {
+    const [url, options] = args;
+    try {
+      const res = await request(url, init?.(options) || options);
+      if (await isModified(res.xhr, url)) {
+        const result = transform ? await transform(res, ...args) : res.data;
+        await this.set(url, result);
+      }
+    } catch (err) {
+      if (process.env.DEBUG) console.error(`Error fetching: ${url}`, err);
+      throw err;
+    } finally {
+      delete requests[url];
+    }
+  }
+}
+
+async function isModified(xhr, url) {
+  const mod = xhr.getResponseHeader('etag')?.trim()
+  || +new Date(xhr.getResponseHeader('last-modified'))
+  || +new Date(xhr.getResponseHeader('date'));
+  if (!mod || mod !== await storage.mod.getOne(url)) {
+    if (mod) await storage.mod.set(url, mod);
+    return true;
+  }
+}

src/background/utils/update.js

@@ -1,8 +1,6 @@
-import {
-  i18n, request, compareVersion, sendCmd,
-} from '#/common';
+import { i18n, request, compareVersion, sendCmd, trueJoin } from '#/common';
 import { CMD_SCRIPT_UPDATE } from '#/common/consts';
-import { getScriptById, getScripts, parseScript } from './db';
+import { fetchResources, getScriptById, getScripts, parseScript } from './db';
 import { parseMeta } from './script';
 import { getOption, setOption } from './options';
 import { commands, notify } from './message';
@@ -25,15 +23,6 @@ const processes = {};
 const NO_HTTP_CACHE = {
   'Cache-Control': 'no-cache, no-store, must-revalidate',
 };
-const OPTIONS = {
-  meta: {
-    headers: { ...NO_HTTP_CACHE, Accept: 'text/x-userscript-meta' },
-  },
-  script: {
-    headers: NO_HTTP_CACHE,
-  },
-};
-
 // resolves to true if successfully updated
 export default function checkUpdate(script) {
   const { id } = script.props;
@@ -43,61 +32,63 @@ export default function checkUpdate(script) {
 
 async function doCheckUpdate(script) {
   const { id } = script.props;
+  let msgOk;
+  let msgErr;
+  let resourceOpts;
   try {
     const { update } = await parseScript({
       id,
       code: await downloadUpdate(script),
       update: { checking: false },
     });
-    if (canNotify(script)) {
+    msgOk = canNotify(script) && i18n('msgScriptUpdated', [update.meta.name || i18n('labelNoName')]);
+    resourceOpts = { headers: NO_HTTP_CACHE };
+    return true;
+  } catch (update) {
+    // Either proceed with normal fetch on no-update or skip it altogether on error
+    resourceOpts = !update.error && !update.checking && {};
+    if (process.env.DEBUG) console.error(update);
+  } finally {
+    if (resourceOpts) {
+      msgErr = await fetchResources(script, null, resourceOpts);
+      if (process.env.DEBUG && msgErr) console.error(msgErr);
+    }
+    if (msgOk || msgErr) {
       notify({
         title: i18n('titleScriptUpdated'),
-        body: i18n('msgScriptUpdated', [update.meta.name || i18n('labelNoName')]),
+        body: [msgOk, msgErr]::trueJoin('\n'),
       });
     }
-    return true;
-  } catch (error) {
-    if (process.env.DEBUG) console.error(error);
-  } finally {
     delete processes[id];
   }
 }
 
-async function downloadUpdate(script) {
-  const downloadURL = (
-    script.custom.downloadURL
-    || script.meta.downloadURL
-    || script.custom.lastInstallURL
-  );
-  const updateURL = (
-    script.custom.updateURL
-    || script.meta.updateURL
-    || downloadURL
-  );
+async function downloadUpdate({ props: { id }, meta, custom }) {
+  const downloadURL = custom.downloadURL || meta.downloadURL || custom.lastInstallURL;
+  const updateURL = custom.updateURL || meta.updateURL || downloadURL;
   if (!updateURL) throw false;
-  let checkingMeta = true;
+  let errorMessage;
   const update = {};
-  const result = { update, where: { id: script.props.id } };
+  const result = { update, where: { id } };
   announce(i18n('msgCheckingForUpdate'));
   try {
-    const { data } = await request(updateURL, OPTIONS.meta);
-    const meta = parseMeta(data);
-    if (compareVersion(script.meta.version, meta.version) >= 0) {
+    const { data } = await request(updateURL, {
+      headers: { ...NO_HTTP_CACHE, Accept: 'text/x-userscript-meta' },
+    });
+    const { version } = parseMeta(data);
+    if (compareVersion(meta.version, version) >= 0) {
       announce(i18n('msgNoUpdate'), { checking: false });
     } else if (!downloadURL) {
       announce(i18n('msgNewVersion'), { checking: false });
     } else {
       announce(i18n('msgUpdating'));
-      checkingMeta = false;
-      return (await request(downloadURL, OPTIONS.script)).data;
+      errorMessage = i18n('msgErrorFetchingScript');
+      return (await request(downloadURL, { headers: NO_HTTP_CACHE })).data;
     }
   } catch (error) {
-    announce(
-      checkingMeta ? i18n('msgErrorFetchingUpdateInfo') : i18n('msgErrorFetchingScript'),
-      { error },
-    );
+    announce(errorMessage || i18n('msgErrorFetchingUpdateInfo'), { error });
   }
-  throw update.error;
+  throw update;
   function announce(message, { error, checking = !error } = {}) {
     Object.assign(update, {
       message,

src/common/index.js

@@ -155,3 +155,7 @@ export function makePause(ms) {
     ? Promise.resolve()
     : new Promise(resolve => setTimeout(resolve, ms));
 }
+
+export function trueJoin(separator) {
+  return this.filter(Boolean).join(separator);
+}

src/common/storage.js

@@ -1,5 +1,5 @@
 import { browser } from './consts';
-import { request, buffer2string, ensureArray } from './util';
+import { ensureArray } from './util';
 
 const base = {
   prefix: '',
@@ -28,7 +28,9 @@ const base = {
       : Promise.resolve();
   },
   removeMulti(ids) {
-    return browser.storage.local.remove(ids.map(this.getKey, this));
+    return ids.length
+      ? browser.storage.local.remove(ids.map(this.getKey, this))
+      : Promise.resolve();
   },
   async dump(data) {
     const output = !this.prefix
@@ -42,24 +44,6 @@ const base = {
   },
 };
 
-const cacheOrFetch = (handle) => {
-  const requests = {};
-  return function cachedHandle(url, ...args) {
-    let promise = requests[url];
-    if (!promise) {
-      promise = handle.call(this, url, ...args)
-      .catch((err) => {
-        console.error(`Error fetching: ${url}`, err);
-      })
-      .then(() => {
-        delete requests[url];
-      });
-      requests[url] = promise;
-    }
-    return promise;
-  };
-};
-
 export default {
 
   base,
@@ -67,19 +51,6 @@ export default {
   cache: {
     ...base,
     prefix: 'cac:',
-    fetch: cacheOrFetch(async function fetch(uri, check) {
-      const { data: buffer, xhr } = await request(uri, { responseType: 'arraybuffer' });
-      const contentType = (xhr.getResponseHeader('content-type') || '').split(';')[0];
-      const data = {
-        contentType,
-        buffer,
-        blob: options => new Blob([buffer], Object.assign({ type: contentType }, options)),
-        string: () => buffer2string(buffer),
-        base64: () => window.btoa(data.string()),
-      };
-      if (check) await check(data);
-      return this.set(uri, `${contentType},${data.base64()}`);
-    }),
   },
 
   code: {
@@ -87,12 +58,15 @@ export default {
     prefix: 'code:',
   },
 
+  // last-modified HTTP header value per URL
+  mod: {
+    ...base,
+    prefix: 'mod:',
+  },
+
   require: {
     ...base,
     prefix: 'req:',
-    fetch: cacheOrFetch(async function fetch(uri) {
-      return this.set(uri, (await request(uri)).data);
-    }),
   },
 
   script: {

src/confirm/views/app.vue

@@ -194,7 +194,7 @@ export default {
         url: this.info.url,
         from: this.info.from,
         require: this.require,
-        resources: this.resources,
+        cache: this.resources,
       })
       .then((result) => {
         this.message = `${result.update.message}[${this.getTimeString()}]`;

src/options/index.js

@@ -95,6 +95,7 @@ function initMain() {
       const index = store.scripts.findIndex(item => item.props.id === data.where.id);
       if (index >= 0) {
         const updated = Object.assign({}, store.scripts[index], data.update);
+        if (updated.error && !data.update.error) updated.error = null;
         Vue.set(store.scripts, index, updated);
         initScript(updated);
       }

src/options/utils/index.js

@@ -22,9 +22,12 @@ export function showMessage(message) {
     // TODO: implement proper keyboard navigation, autofocus, and Enter/Esc in Modal module
     document.querySelector('.vl-modal button').focus();
   } else {
-    setTimeout(() => {
-      modal.close();
-    }, 2000);
+    const timer = setInterval(() => {
+      if (!document.querySelector('.vl-modal .modal-content:hover')) {
+        clearInterval(timer);
+        modal.close();
+      }
+    }, message.timeout || 2000);
   }
 }
 

src/options/views/edit/index.vue

@@ -133,6 +133,12 @@ export default {
     code() {
       this.canSave = true;
     },
+    // usually errors for resources
+    'initial.error'(error) {
+      if (error) {
+        showMessage({ text: `${this.initial.message}\n\n${error}` });
+      }
+    },
     settings: {
       deep: true,
       handler() {

src/options/views/message.vue

@@ -1,5 +1,5 @@
 <template>
-  <div class="message modal-content">
+  <div class="message modal-content" :class="{ multiline: /\n/.test(message.text) }">
     <div class="mb-1" v-if="message.text" v-text="message.text"></div>
     <form v-if="message.buttons" @submit.prevent>
       <input class="mb-1" type="text" v-if="message.input !== false" v-model="message.input">
@@ -67,9 +67,19 @@ export default {
 <style>
 .message {
   width: 18rem;
+  white-space: pre-wrap;
+  overflow-wrap: break-word;
   border-bottom-left-radius: .2rem;
   border-bottom-right-radius: .2rem;
   box-shadow: 0 0 .2rem rgba(0,0,0,.2);
+  &.multiline {
+    width: auto;
+    max-width: 50vw;
+    &::first-line {
+      font-weight: bold;
+      text-decoration: underline;
+    }
+  }
   input {
     width: 100%;
   }