Support async stack traces #1621
Conversation
|
Sorry, I cant merge this. The query method is a nasty hack. I can merge the change you suggested in the db-errors library as I mentioned in the issue. |
|
It's true it's a hack, so I completely understand :) That said, you can't fix this issue within db-errors. Custom promise implementations simply won't get the stack trace when await-ed, as I mention here. Here's a monkey patch if anyone is interested: import Knex from "knex";
import { Model, Transaction, QueryBuilder } from "objection";
class QueryBuilderWAsyncTrace<A extends Model, B, C> extends QueryBuilder<A, B, C> {
_asyncStack: any;
async execute() {
let p;
try {
p = await super.execute();
} catch (err) {
if (this._asyncStack) {
const originalStack = err.stack.split("\n");
const newStack = originalStack.concat(this._asyncStack);
err.stack = newStack.join("\n");
}
throw err;
}
return p;
}
}
const ModelWithAsyncTrace = <M extends typeof Model>(ModelClass: M): M => {
return class extends Model {
static query<T extends Model>(trx?: Transaction | Knex<any, any[]>) {
const query = (QueryBuilderWAsyncTrace.forClass(this as any).transacting(
trx as any,
) as unknown) as QueryBuilderWAsyncTrace<T, T[], T[]>;
const knex = this.knex();
if (knex?.client?.config?.asyncStackTraces) {
const err = new Error();
if (err.stack) {
const lines = err.stack.split("\n").slice(1);
query._asyncStack = lines;
}
}
(this as any).onCreateQuery(query);
return query;
}
} as any; // Not gonna bother trying to type this one... trust me.
};
export default ModelWithAsyncTrace;To use, simply extend |
|
So you are still using objection 1.x? Have you tried 2.0? It uses native promises, which have async stack traces out of the box. There's still some old-fashioned promise code in the codebase which could cut the stack trace, but those could be fixed bu migrating that code to use async/await. |
|
Yup, I tried 2.0 and it didn't help. I think any custom promise implementation won't get the full stack trace. For example, QueryBuilder looks like: class CustomPromise {
async execute() {
throw new Error('Uh-oh');
}
then(...args) {
return this.execute().then(...args);
}
catch(...args) {
return this.execute().catch(...args);
}
}
async function main() {
await new CustomPromise();
}
main().catch(e => {
console.log(e);
});This code will output: Note that This is why Knex follows the stack-copying strategy: knex/knex#2500 |
|
@rclmenezes thanks for that monkey patch. I was maintaining my own fork of objection.js for the very same reason. Do you know if it works well with objection 2.0 ? |
|
@koskimas maybe then objection.js should consider for the future majors not mimicking the promise API in the QueryBuilder and always require the explicit Or what other solution do you see to get the correct stack traces? |
|
so I tested it now and works like a charm. Stack without it: same error with the monkey patch: You just made my day @rclmenezes. Finally I can deprecate my fork. |
|
Are we ever getting this? |
|
This is still needed! Even with newest knex, objection, node v15, async stack traces aren't visible. Well explained in #1621 (comment) and also that workaround works. Just remember to also pass |
|
I improved a little bit the monkey patch of @rclmenezes to make it work in TS with newest objection and also with other mixins, if e.g. you're using import type Knex from "knex";
import type { Model, Transaction } from "objection";
// Needed to make async error stack traces work
// https://github.com/Vincit/objection.js/pull/1621#issuecomment-567220373
export const ModelWithAsyncStackTrace = <TModelClass extends typeof Model>(
ModelClass: TModelClass
): TModelClass => {
class QueryBuilderWithAsyncStackTrace<
TModel extends Model,
R = TModel[]
> extends ModelClass.QueryBuilder<TModel, R> {
_asyncStack: any;
async execute() {
let p;
try {
p = await super.execute();
} catch (error) {
if (this._asyncStack) {
const originalStack = error.stack.split("\n");
const newStack = originalStack.concat(this._asyncStack);
error.stack = newStack.join("\n");
}
throw error;
}
return p;
}
}
return class ModelWithAsyncStackTrace extends (ModelClass as any) {
static get QueryBuilder() {
return QueryBuilderWithAsyncStackTrace;
}
static query<T extends Model>(trx?: Transaction | Knex<any, any[]>) {
const query = (this.QueryBuilder.forClass(this as any).transacting(
trx as any
) as unknown) as QueryBuilderWithAsyncStackTrace<T, T[]>;
const knex = this.knex();
if (knex?.client?.config?.asyncStackTraces) {
const err = new Error();
if (err.stack) {
const lines = err.stack.split("\n").slice(2);
query._asyncStack = lines;
}
}
(this as any).onCreateQuery(query);
return query;
}
} as any; // Not gonna bother trying to type this one... trust me.
};
// Usage
export abstract class BaseModel extends ModelWithAsyncStackTrace(cursor(Model)) {
} |
|
Objection is a wonderful project, but without the proper stack trace, it makes super trick to debug. :( |
|
As someone dealing with this frequently, I'd be very excited to see some openness toward fixing this! |
Rough draft implementation of a fix for #1618 (comment)