From 3af9a14a0e78be88c5a048b79187c32796c06a7c Mon Sep 17 00:00:00 2001 From: Ven Date: Sun, 30 Oct 2022 02:58:11 +0100 Subject: Patcher: More useful errors with code diffs (#177) * Patcher: More useful errors with code diffs * Nicer log formatting * PluginCards: ellipsises --- package.json | 2 ++ pnpm-lock.yaml | 13 ++++++++ scripts/build/build.mjs | 9 ++--- scripts/build/buildWeb.mjs | 5 +-- src/components/PluginSettings/index.tsx | 14 +++++++- src/globals.d.ts | 1 + src/utils/logger.ts | 20 ++++++++++-- src/webpack/patchWebpack.ts | 58 ++++++++++++++++++++++++++------- 8 files changed, 102 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 7cab94e..5823ddb 100644 --- a/package.json +++ b/package.json @@ -30,9 +30,11 @@ }, "dependencies": { "console-menu": "^0.1.0", + "diff": "^5.1.0", "fflate": "^0.7.4" }, "devDependencies": { + "@types/diff": "^5.0.2", "@types/node": "^18.7.13", "@types/react": "^18.0.17", "@types/yazl": "^2.4.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 924d0ec..1da6aec 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,11 +1,13 @@ lockfileVersion: 5.4 specifiers: + '@types/diff': ^5.0.2 '@types/node': ^18.7.13 '@types/react': ^18.0.17 '@types/yazl': ^2.4.2 '@typescript-eslint/parser': ^5.39.0 console-menu: ^0.1.0 + diff: ^5.1.0 discord-types: ^1.3.26 esbuild: ^0.15.5 eslint: ^8.24.0 @@ -19,9 +21,11 @@ specifiers: dependencies: console-menu: 0.1.0 + diff: 5.1.0 fflate: 0.7.4 devDependencies: + '@types/diff': 5.0.2 '@types/node': 18.7.13 '@types/react': 18.0.17 '@types/yazl': 2.4.2 @@ -109,6 +113,10 @@ packages: fastq: 1.13.0 dev: true + /@types/diff/5.0.2: + resolution: {integrity: sha512-uw8eYMIReOwstQ0QKF0sICefSy8cNO/v7gOTiIy9SbwuHyEecJUm7qlgueOO5S1udZ5I/irVydHVwMchgzbKTg==} + dev: true + /@types/node/18.7.13: resolution: {integrity: sha512-46yIhxSe5xEaJZXWdIBP7GU4HDTG8/eo0qd9atdiL+lFpA03y8KS+lkTN834TWJj5767GbWv4n/P6efyTFt1Dw==} dev: true @@ -333,6 +341,11 @@ packages: resolution: {integrity: sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ==} dev: true + /diff/5.1.0: + resolution: {integrity: sha512-D+mk+qE8VC/PAUrlAU34N+VfXev0ghe5ywmpqrawphmVZc1bEfn56uo9qpyGp1p4xpzOHkSW4ztBd6L7Xx4ACw==} + engines: {node: '>=0.3.1'} + dev: false + /dir-glob/3.0.1: resolution: {integrity: sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==} engines: {node: '>=8'} diff --git a/scripts/build/build.mjs b/scripts/build/build.mjs index bbf8105..82264ee 100755 --- a/scripts/build/build.mjs +++ b/scripts/build/build.mjs @@ -19,10 +19,11 @@ import esbuild from "esbuild"; -import { commonOpts, gitHash, globPlugins, isStandalone } from "./common.mjs"; +import { commonOpts, gitHash, globPlugins, isStandalone, watch } from "./common.mjs"; const defines = { - IS_STANDALONE: isStandalone + IS_STANDALONE: isStandalone, + IS_DEV: JSON.stringify(watch) }; if (defines.IS_STANDALONE === "false") // If this is a local build (not standalone), optimise @@ -81,8 +82,8 @@ await Promise.all([ ...commonOpts.plugins ], define: { - IS_WEB: "false", - IS_STANDALONE: isStandalone + ...defines, + IS_WEB: false } }), ]).catch(err => { diff --git a/scripts/build/buildWeb.mjs b/scripts/build/buildWeb.mjs index f0197b7..1efdd8f 100755 --- a/scripts/build/buildWeb.mjs +++ b/scripts/build/buildWeb.mjs @@ -26,7 +26,7 @@ import { join } from "path"; // wtf is this assert syntax import PackageJSON from "../../package.json" assert { type: "json" }; -import { commonOpts, fileIncludePlugin, gitHashPlugin, gitRemotePlugin, globPlugins } from "./common.mjs"; +import { commonOpts, fileIncludePlugin, gitHashPlugin, gitRemotePlugin, globPlugins, watch } from "./common.mjs"; /** * @type {esbuild.BuildOptions} @@ -46,7 +46,8 @@ const commonOptions = { target: ["esnext"], define: { IS_WEB: "true", - IS_STANDALONE: "true" + IS_STANDALONE: "true", + IS_DEV: JSON.stringify(watch) } }; diff --git a/src/components/PluginSettings/index.tsx b/src/components/PluginSettings/index.tsx index 6ad9750..1a2e78f 100644 --- a/src/components/PluginSettings/index.tsx +++ b/src/components/PluginSettings/index.tsx @@ -145,7 +145,19 @@ function PluginCard({ plugin, disabled, onRestartNeeded, onMouseEnter, onMouseLe onChange={toggleEnabled} disabled={disabled} value={isEnabled()} - note={{plugin.description}} + note={ + {plugin.description} + } hideBorder={true} > diff --git a/src/globals.d.ts b/src/globals.d.ts index 071bca2..2872f62 100644 --- a/src/globals.d.ts +++ b/src/globals.d.ts @@ -32,6 +32,7 @@ declare global { * replace: `${IS_WEB}?foo:bar` */ export var IS_WEB: boolean; + export var IS_DEV: boolean; export var IS_STANDALONE: boolean; export var VencordNative: typeof import("./VencordNative").default; diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 309f4db..24cd2c8 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -17,11 +17,23 @@ */ export default class Logger { + /** + * Returns the console format args for a title with the specified background colour and black text + * @param color Background colour + * @param title Text + * @returns Array. Destructure this into {@link Logger}.errorCustomFmt or console.log + * + * @example logger.errorCustomFmt(...Logger.makeTitleElements("white", "Hello"), "World"); + */ + static makeTitle(color: string, title: string): [string, ...string[]] { + return ["%c %c %s ", "", `background: ${color}; color: black; font-weight: bold; border-radius: 5px;`, title]; + } + constructor(public name: string, public color: string) { } - private _log(level: "log" | "error" | "warn" | "info" | "debug", levelColor: string, args: any[]) { + private _log(level: "log" | "error" | "warn" | "info" | "debug", levelColor: string, args: any[], customFmt = "") { console[level]( - `%c Vencord %c %c ${this.name} `, + `%c Vencord %c %c ${this.name} ${customFmt}`, `background: ${levelColor}; color: black; font-weight: bold; border-radius: 5px;`, "", `background: ${this.color}; color: black; font-weight: bold; border-radius: 5px;` @@ -41,6 +53,10 @@ export default class Logger { this._log("error", "#e78284", args); } + public errorCustomFmt(fmt: string, ...args: any[]) { + this._log("error", "#e78284", args, fmt); + } + public warn(...args: any[]) { this._log("warn", "#e5c890", args); } diff --git a/src/webpack/patchWebpack.ts b/src/webpack/patchWebpack.ts index f679f3a..5596730 100644 --- a/src/webpack/patchWebpack.ts +++ b/src/webpack/patchWebpack.ts @@ -41,7 +41,7 @@ Object.defineProperty(window, WEBPACK_CHUNK, { }); function patchPush() { - function handlePush(chunk) { + function handlePush(chunk: any) { try { const modules = chunk[1]; const { subscriptions, listeners } = Vencord.Webpack; @@ -56,7 +56,7 @@ function patchPush() { // Additionally, `[actual newline]` is one less char than "\n", so if Discord // ever targets newer browsers, the minifier could potentially use this trick and // cause issues. - let code = mod.toString().replaceAll("\n", ""); + let code: string = mod.toString().replaceAll("\n", ""); const originalMod = mod; const patchedBy = new Set(); @@ -90,6 +90,7 @@ function patchPush() { logger.error("Error in webpack listener", err); } } + for (const [filter, callback] of subscriptions) { try { if (filter(exports)) { @@ -113,45 +114,80 @@ function patchPush() { } } }; + modules[id].toString = () => mod.toString(); modules[id].original = originalMod; for (let i = 0; i < patches.length; i++) { const patch = patches[i]; if (patch.predicate && !patch.predicate()) continue; + if (code.includes(patch.find)) { patchedBy.add(patch.plugin); + // @ts-ignore we change all patch.replacement to array in plugins/index for (const replacement of patch.replacement) { const lastMod = mod; const lastCode = code; + try { const newCode = code.replace(replacement.match, replacement.replace); if (newCode === code) { - logger.warn(`Patch by ${patch.plugin} had no effect: ${replacement.match}`); - logger.debug("Function Source:\n", code); + logger.warn(`Patch by ${patch.plugin} had no effect (Module id is ${id}): ${replacement.match}`); + if (IS_DEV) { + logger.debug("Function Source:\n", code); + } } else { code = newCode; mod = (0, eval)(`// Webpack Module ${id} - Patched by ${[...patchedBy].join(", ")}\n${newCode}\n//# sourceURL=WebpackModule${id}`); } } catch (err) { - // TODO - More meaningful errors. This probably means moving away from string.replace - // in favour of manual matching. Then cut out the context and log some sort of - // diff - logger.error("Failed to apply patch of", patch.plugin, err); - logger.debug("Original Source\n", lastCode); - logger.debug("Patched Source\n", code); + logger.error(`Failed to apply patch ${replacement.match} of ${patch.plugin} to ${id}:\n`, err); + + if (IS_DEV) { + const changeSize = code.length - lastCode.length; + const match = lastCode.match(replacement.match)!; + + // Use 200 surrounding characters of context + const start = Math.max(0, match.index! - 200); + const end = Math.min(lastCode.length, match.index! + match[0].length + 200); + // (changeSize may be negative) + const endPatched = end + changeSize; + + const context = lastCode.slice(start, end); + const patchedContext = code.slice(start, endPatched); + + // inline require to avoid including it in !IS_DEV builds + const diff = (require("diff") as typeof import("diff")).diffWordsWithSpace(context, patchedContext); + let fmt = "%c %s "; + const elements = [] as string[]; + for (const d of diff) { + const color = d.removed + ? "red" + : d.added + ? "lime" + : "grey"; + fmt += "%c%s"; + elements.push("color:" + color, d.value); + } + + logger.errorCustomFmt(...Logger.makeTitle("white", "Before"), context); + logger.errorCustomFmt(...Logger.makeTitle("white", "After"), context); + const [titleFmt, ...titleElements] = Logger.makeTitle("white", "Diff"); + logger.errorCustomFmt(titleFmt + fmt, ...titleElements, ...elements); + } code = lastCode; mod = lastMod; patchedBy.delete(patch.plugin); } } + if (!patch.all) patches.splice(i--, 1); } } } } catch (err) { - logger.error("oopsie", err); + logger.error("Error in handlePush", err); } return handlePush.original.call(window[WEBPACK_CHUNK], chunk); -- cgit