From 34de8c4f268c1e657c780cfe13ace1e7feeacf53 Mon Sep 17 00:00:00 2001 From: dfahlander Date: Mon, 22 Jan 2024 10:57:34 +0100 Subject: [PATCH 1/4] Fail fast on Table.get(undefined) It's a common pitfall to do db.someTable.get(someVariable) without checking that someVariable isn't null. This is a common pitfall and in the case the call to Table.get() is the initial db call that the application makes, the call stack will be totally unreadable because: 1. Table.get() will first asynchronically auto-open the database. 2. Whe DB is finally opened it will fail deep inside DBCore leaving the user with an unreadable call stack. See https://github.com/dexie/Dexie.js/issues/1806#issuecomment-1884977367 --- src/classes/table/table.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/classes/table/table.ts b/src/classes/table/table.ts index 151389367..19e5cfedc 100644 --- a/src/classes/table/table.ts +++ b/src/classes/table/table.ts @@ -78,6 +78,7 @@ export class Table implements ITable { get(keyOrCrit, cb?) { if (keyOrCrit && keyOrCrit.constructor === Object) return this.where(keyOrCrit as { [key: string]: IndexableType }).first(cb); + if (keyOrCrit == null) return rejection(new exceptions.Type(`Invalid argument to Table.get()`)); return this._trans('readonly', (trans) => { return this.core.get({trans, key: keyOrCrit}) From 839393aae6803190445c44ee27a572871ada8dd6 Mon Sep 17 00:00:00 2001 From: dfahlander Date: Mon, 22 Jan 2024 11:06:27 +0100 Subject: [PATCH 2/4] Cleanse out old "long stacks" support. Use Chrome's console.createTask() instead. We used to have own special support for long async stacks to simplify debugging. However, now all browsers already have long async stacks supported in devtools. This commit removes all special code that deals with long stack support. It also introduces the usage of Chrome's Async Stack Tagging API (console.createTask()) that makes it easier to follow console traces on exceptions and find the application code that initiated the operation. Also, when Dexie.debug is true, we trace all errors from dexie operations with the help of async stack tagging, hopefully making it easier for app developers to find issues in app code. See #1806 --- src/classes/table/table.ts | 14 ++++- src/errors/errors.js | 11 ---- src/helpers/debug.ts | 37 +----------- src/helpers/promise.js | 109 +++-------------------------------- test/dexie-unittest-utils.js | 11 +--- 5 files changed, 21 insertions(+), 161 deletions(-) diff --git a/src/classes/table/table.ts b/src/classes/table/table.ts index 19e5cfedc..e081a4d98 100644 --- a/src/classes/table/table.ts +++ b/src/classes/table/table.ts @@ -39,11 +39,13 @@ export class Table implements ITable { { const trans: Transaction = this._tx || PSD.trans; const tableName = this.name; + // @ts-ignore: Use Chrome's Async Stack Tagging API to allow tracing and simplify debugging for dexie users. + const task = debug && typeof console !== 'undefined' && console.createTask && console.createTask(`Dexie: ${mode === 'readonly' ? 'read' : 'write' } ${this.name}`); function checkTableInTransaction(resolve, reject, trans: Transaction) { if (!trans.schema[tableName]) throw new exceptions.NotFound("Table " + tableName + " not part of transaction"); - return fn(trans.idbtrans, trans); + return fn(trans.idbtrans, trans) as Promise; } // Surround all in a microtick scope. // Reason: Browsers (modern Safari + older others) @@ -60,11 +62,19 @@ export class Table implements ITable { // in native engine. const wasRootExec = beginMicroTickScope(); try { - return trans && trans.db._novip === this.db._novip ? + let p = trans && trans.db._novip === this.db._novip ? trans === PSD.trans ? trans._promise(mode, checkTableInTransaction, writeLocked) : newScope(() => trans._promise(mode, checkTableInTransaction, writeLocked), { trans: trans, transless: PSD.transless || PSD }) : tempTransaction(this.db, mode, [this.name], checkTableInTransaction); + if (task) { // Dexie.debug = true so we trace errors + p._consoleTask = task; + p = p.catch(err => { + console.trace(err); + return rejection(err); + }); + } + return p; } finally { if (wasRootExec) endMicroTickScope(); } diff --git a/src/errors/errors.js b/src/errors/errors.js index abe0ad6a0..4d22d31d8 100644 --- a/src/errors/errors.js +++ b/src/errors/errors.js @@ -1,5 +1,4 @@ import { derive, setProp } from '../functions/utils'; -import { getErrorWithStack, prettyStack } from '../helpers/debug'; var dexieErrorNames = [ 'Modify', @@ -56,18 +55,11 @@ export function DexieError (name, msg) { // 2. It doesn't give us much in this case. // 3. It would require sub classes to call super(), which // is not needed when deriving from Error. - this._e = getErrorWithStack(); this.name = name; this.message = msg; } derive(DexieError).from(Error).extend({ - stack: { - get: function() { - return this._stack || - (this._stack = this.name + ": " + this.message + prettyStack(this._e, 2)); - } - }, toString: function(){ return this.name + ": " + this.message; } }); @@ -83,7 +75,6 @@ function getMultiErrorMessage (msg, failures) { // Specific constructor because it contains members failures and failedKeys. // export function ModifyError (msg, failures, successCount, failedKeys) { - this._e = getErrorWithStack(); this.failures = failures; this.failedKeys = failedKeys; this.successCount = successCount; @@ -92,7 +83,6 @@ export function ModifyError (msg, failures, successCount, failedKeys) { derive(ModifyError).from(DexieError); export function BulkError (msg, failures) { - this._e = getErrorWithStack(); this.name = "BulkError"; this.failures = Object.keys(failures).map(pos => failures[pos]); this.failuresByPos = failures; @@ -122,7 +112,6 @@ export var exceptions = errorList.reduce((obj,name)=>{ // 'eval-evil'. var fullName = name + "Error"; function DexieError (msgOrInner, inner){ - this._e = getErrorWithStack(); this.name = fullName; if (!msgOrInner) { this.message = defaultTexts[name] || fullName; diff --git a/src/helpers/debug.ts b/src/helpers/debug.ts index d54d70c15..be60fff76 100644 --- a/src/helpers/debug.ts +++ b/src/helpers/debug.ts @@ -6,46 +6,11 @@ export var debug = typeof location !== 'undefined' && export function setDebug(value, filter) { debug = value; - libraryFilter = filter; } -export var libraryFilter = () => true; - -export const NEEDS_THROW_FOR_STACK = !new Error("").stack; - -export function getErrorWithStack() { - "use strict"; - if (NEEDS_THROW_FOR_STACK) try { - // Doing something naughty in strict mode here to trigger a specific error - // that can be explicitely ignored in debugger's exception settings. - // If we'd just throw new Error() here, IE's debugger's exception settings - // will just consider it as "exception thrown by javascript code" which is - // something you wouldn't want it to ignore. - getErrorWithStack.arguments; - throw new Error(); // Fallback if above line don't throw. - } catch(e) { - return e; - } - return new Error(); -} - -export function prettyStack(exception, numIgnoredFrames) { - var stack = exception.stack; - if (!stack) return ""; - numIgnoredFrames = (numIgnoredFrames || 0); - if (stack.indexOf(exception.name) === 0) - numIgnoredFrames += (exception.name + exception.message).split('\n').length; - return stack.split('\n') - .slice(numIgnoredFrames) - .filter(libraryFilter) - .map(frame => "\n" + frame) - .join(''); -} - -// TODO: Replace this in favor of a decorator instead. export function deprecated (what: string, fn: (...args)=>T) { return function () { - console.warn(`${what} is deprecated. See https://dexie.org/docs/Deprecations. ${prettyStack(getErrorWithStack(), 1)}`); + console.warn(`${what} is deprecated. See https://dexie.org/docs/Deprecations}`); return fn.apply(this, arguments); } as (...args)=>T } diff --git a/src/helpers/promise.js b/src/helpers/promise.js index b0f98f0ae..f971a2888 100644 --- a/src/helpers/promise.js +++ b/src/helpers/promise.js @@ -6,7 +6,7 @@ import { _global } from '../globals/global'; import {tryCatch, props, setProp, getPropertyDescriptor, getArrayOf, extend, getProto} from '../functions/utils'; import {nop, callBoth, mirror} from '../functions/chaining-functions'; -import {debug, prettyStack, getErrorWithStack} from './debug'; +import {debug} from './debug'; import {exceptions} from '../errors'; // @@ -20,7 +20,6 @@ import {exceptions} from '../errors'; // another strategy now that simplifies everything a lot: to always execute callbacks in a new micro-task, but have an own micro-task // engine that is indexedDB compliant across all browsers. // Promise class has also been optimized a lot with inspiration from bluebird - to avoid closures as much as possible. -// Also with inspiration from bluebird, asyncronic stacks in debug mode. // // Specific non-standard features of this Promise class: // * Custom zone support (a.k.a. PSD) with ability to keep zones also when using native promises as well as @@ -36,11 +35,7 @@ import {exceptions} from '../errors'; // Used in Promise constructor to emulate a private constructor. var INTERNAL = {}; -// Async stacks (long stacks) must not grow infinitely. const - LONG_STACKS_CLIP_LIMIT = 100, - // When calling error.stack or promise.stack, limit the number of asyncronic stacks to print out. - MAX_LONG_STACKS = 20, ZONE_ECHO_LIMIT = 100, [resolvedNativePromise, nativePromiseProto, resolvedGlobalPromise] = typeof Promise === 'undefined' ? [] : @@ -61,8 +56,6 @@ const export const NativePromise = resolvedNativePromise && resolvedNativePromise.constructor; const patchGlobalPromise = !!resolvedGlobalPromise; -var stack_being_generated = false; - /* The default function used only for the very first promise in a promise chain. As soon as then promise is resolved or rejected, all next tasks will be executed in micro ticks emulated in this module. For indexedDB compatibility, this means that every method needs to @@ -91,7 +84,6 @@ var isOutsideMicroTick = true, // True when NOT in a virtual microTick. needsNewPhysicalTick = true, // True when a push to microtickQueue must also schedulePhysicalTick() unhandledErrors = [], // Rejected promises that has occured. Used for triggering 'unhandledrejection'. rejectingErrors = [], // Tracks if errors are being re-rejected during onRejected callback. - currentFulfiller = null, rejectionMapper = mirror; // Remove in next major when removing error mapping of DOMErrors and DOMExceptions export var globalPSD = { @@ -124,12 +116,6 @@ export default function DexiePromise(fn) { this._lib = false; // Current async scope var psd = (this._PSD = PSD); - - if (debug) { - this._stackHolder = getErrorWithStack(); - this._prev = null; - this._numPrev = 0; // Number of previous promises (for long stacks) - } if (typeof fn !== 'function') { if (fn !== INTERNAL) throw new TypeError('Not a function'); @@ -164,7 +150,7 @@ const thenProp = { reject, psd)); }); - debug && linkToPreviousPromise(rv, this); + if (this._consoleTask) rv._consoleTask = this._consoleTask; return rv; } @@ -218,21 +204,6 @@ props(DexiePromise.prototype, { }); }, - stack: { - get: function() { - if (this._stack) return this._stack; - try { - stack_being_generated = true; - var stacks = getStack (this, [], MAX_LONG_STACKS); - var stack = stacks.join("\nFrom previous: "); - if (this._state !== null) this._stack = stack; // Stack may be updated on reject. - return stack; - } finally { - stack_being_generated = false; - } - } - }, - timeout: function (ms, msg) { return ms < Infinity ? new DexiePromise((resolve, reject) => { @@ -278,7 +249,6 @@ props (DexiePromise, { value.then(resolve, reject); }); var rv = new DexiePromise(INTERNAL, true, value); - linkToPreviousPromise(rv, currentFulfiller); return rv; }, @@ -402,18 +372,6 @@ function handleRejection (promise, reason) { reason = rejectionMapper(reason); promise._state = false; promise._value = reason; - debug && reason !== null && typeof reason === 'object' && !reason._promise && tryCatch(()=>{ - var origProp = getPropertyDescriptor(reason, "stack"); - reason._promise = promise; - setProp(reason, "stack", { - get: () => - stack_being_generated ? - origProp && (origProp.get ? - origProp.get.apply(reason) : - origProp.value) : - promise.stack - }); - }); // Add the failure to a list of possibly uncaught errors addPossiblyUnhandledError(promise); propagateAllListeners(promise); @@ -460,70 +418,25 @@ function propagateToListener(promise, listener) { function callListener (cb, promise, listener) { try { - // Set static variable currentFulfiller to the promise that is being fullfilled, - // so that we connect the chain of promises (for long stacks support) - currentFulfiller = promise; - // Call callback and resolve our listener with it's return value. var ret, value = promise._value; - if (promise._state) { - // cb is onResolved - ret = cb (value); - } else { - // cb is onRejected - if (rejectingErrors.length) rejectingErrors = []; - ret = cb(value); - if (rejectingErrors.indexOf(value) === -1) - markErrorAsHandled(promise); // Callback didnt do Promise.reject(err) nor reject(err) onto another promise. + if (!promise._state && rejectingErrors.length) rejectingErrors = []; + // cb is onResolved + ret = debug && promise._consoleTask ? promise._consoleTask.run(()=>cb (value)) : cb (value); + if (!promise._state && rejectingErrors.indexOf(value) === -1) { + markErrorAsHandled(promise); // Callback didnt do Promise.reject(err) nor reject(err) onto another promise. } listener.resolve(ret); } catch (e) { // Exception thrown in callback. Reject our listener. listener.reject(e); } finally { - // Restore env and currentFulfiller. - currentFulfiller = null; if (--numScheduledCalls === 0) finalizePhysicalTick(); --listener.psd.ref || listener.psd.finalize(); } } -function getStack (promise, stacks, limit) { - if (stacks.length === limit) return stacks; - var stack = ""; - if (promise._state === false) { - var failure = promise._value, - errorName, - message; - - if (failure != null) { - errorName = failure.name || "Error"; - message = failure.message || failure; - stack = prettyStack(failure, 0); - } else { - errorName = failure; // If error is undefined or null, show that. - message = ""; - } - stacks.push(errorName + (message ? ": " + message : "") + stack); - } - if (debug) { - stack = prettyStack(promise._stackHolder, 2); - if (stack && stacks.indexOf(stack) === -1) stacks.push(stack); - if (promise._prev) getStack(promise._prev, stacks, limit); - } - return stacks; -} - -function linkToPreviousPromise(promise, prev) { - // Support long stacks by linking to previous completed promise. - var numPrev = prev ? prev._numPrev + 1 : 0; - if (numPrev < LONG_STACKS_CLIP_LIMIT) { // Prohibit infinite Promise loops to get an infinite long memory consuming "tail". - promise._prev = prev; - promise._numPrev = numPrev; - } -} - /* The callback to schedule with queueMicrotask(). It runs a virtual microtick and executes any callback registered in microtickQueue. */ @@ -813,14 +726,6 @@ function nativeAwaitCompatibleWrap(fn, zone, possibleAwait, cleanup) { }; } -function getPatchedPromiseThen (origThen, zone) { - return function (onResolved, onRejected) { - return origThen.call(this, - nativeAwaitCompatibleWrap(onResolved, zone), - nativeAwaitCompatibleWrap(onRejected, zone)); - }; -} - /** Execute callback in global context */ export function execInGlobalContext(cb) { if (Promise === NativePromise && task.echoes === 0) { diff --git a/test/dexie-unittest-utils.js b/test/dexie-unittest-utils.js index 2ff46c34f..802755df3 100644 --- a/test/dexie-unittest-utils.js +++ b/test/dexie-unittest-utils.js @@ -17,16 +17,7 @@ config.urlConfig.push(/*{ id: "dontoptimize", label: "Dont optimize tests", tooltip: "Always delete and recreate the DB between each test" -}, { - id: "longstacks", - label: "Long async stacks", - tooltip: "Set Dexie.debug=true, turning on long async stacks on all" + - " errors (Actually we use Dexie.debug='dexie' so that frames from" + - " dexie.js are also included)" - }); - -Dexie.debug = window.location.search.indexOf('longstacks') !== -1 ? 'dexie' : false; -if (window.location.search.indexOf('longstacks=tests') !== -1) Dexie.debug = true; // Don't include stuff from dexie.js. +}); var no_optimize = window.no_optimize || window.location.search.indexOf('dontoptimize') !== -1; From 3c3fd0224c8a5b4612409d851c61eab55c9b4dae Mon Sep 17 00:00:00 2001 From: dfahlander Date: Mon, 22 Jan 2024 11:18:54 +0100 Subject: [PATCH 3/4] Remove polyfills from test page --- test/run-unit-tests.html | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/run-unit-tests.html b/test/run-unit-tests.html index 3693d92b5..02d36e631 100644 --- a/test/run-unit-tests.html +++ b/test/run-unit-tests.html @@ -8,9 +8,6 @@
- - - From 5ced3cce42486c4f3b6df666c33ebad07f7a755e Mon Sep 17 00:00:00 2001 From: dfahlander Date: Mon, 22 Jan 2024 11:34:09 +0100 Subject: [PATCH 4/4] Remove _stackHolder not needed anymore --- src/classes/dexie/dexie-open.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/classes/dexie/dexie-open.ts b/src/classes/dexie/dexie-open.ts index 1a6990412..ac08df40f 100644 --- a/src/classes/dexie/dexie-open.ts +++ b/src/classes/dexie/dexie-open.ts @@ -25,7 +25,6 @@ export function dexieOpen (db: Dexie) { return state.dbReadyPromise.then(() => state.dbOpenError ? rejection (state.dbOpenError) : db); - Debug.debug && (state.openCanceller._stackHolder = Debug.getErrorWithStack()); // Let stacks point to when open() was called rather than where new Dexie() was called. state.isBeingOpened = true; state.dbOpenError = null; state.openComplete = false;