Skip to content

firestore: only pay .stack cost in error paths #9102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rockwotj wants to merge 1 commit into
base: main
Choose a base branch
from
Open

firestore: only pay .stack cost in error paths #9102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/bulk-writer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class BulkCommitBatch extends WriteBatch {
const tag = options?.requestTag ?? requestTag();

// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();
Comment thread
rockwotj marked this conversation as resolved.

let response: api.IBatchWriteResponse;
try {
Expand Down Expand Up @@ -295,7 +295,7 @@ class BulkCommitBatch extends WriteBatch {
status.message || undefined,
);
error.code = status.code as number;
this.pendingOps[i].onError(wrapError(error, stack));
this.pendingOps[i].onError(wrapError(error, callsiteError.stack!));
}
}
},
Expand Down
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1374,15 +1374,15 @@ export class Firestore implements firestore.Firestore {
const tag = requestTag();

// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();

return this.initializeIfNeeded(tag)
.then(() => {
const reader = new DocumentReader(this, documents, fieldMask);
return reader.get(tag);
})
.catch(err => {
throw wrapError(err, stack);
throw wrapError(err, callsiteError.stack!);
});
},
);
Expand Down
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/pipelines/pipeline-util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class ExecutionUtil {
transactionOrReadTime?: Uint8Array | Timestamp | api.ITransactionOptions,
): Promise<PipelineResponse> {
// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();

return new Promise((resolve, reject): void => {
const result: Array<PipelineResult> = [];
Expand All @@ -103,7 +103,7 @@ export class ExecutionUtil {
transactionOrReadTime,
);
stream.on('error', err => {
reject(wrapError(err, stack));
reject(wrapError(err, callsiteError.stack!));
});
stream.on('data', (data: PipelineStreamElement[]) => {
for (const element of data) {
Expand Down
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/reference/aggregate-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export class AggregateQuery<
>
> {
// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();

return new Promise((resolve, reject) => {
const output: QueryResponse<
Expand All @@ -148,7 +148,7 @@ export class AggregateQuery<

const stream = this._stream(transactionOrReadTime, explainOptions);
stream.on('error', err => {
reject(wrapError(err, stack));
reject(wrapError(err, callsiteError.stack!));
});
stream.on(
'data',
Expand Down
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/reference/query-util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class QueryUtil<
explainOptions?: firestore.ExplainOptions,
): Promise<QueryResponse<ReturnType<Template['_createSnapshot']>>> {
// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();

return new Promise((resolve, reject) => {
const docs: Array<QueryDocumentSnapshot<AppModelType, DbModelType>> = [];
Expand All @@ -84,7 +84,7 @@ export class QueryUtil<
explainOptions,
)
.on('error', err => {
reject(wrapError(err, stack));
reject(wrapError(err, callsiteError.stack!));
})
.on('data', (data: QueryStreamElement<AppModelType, DbModelType>) => {
if (data.transaction) {
Expand Down
4 changes: 2 additions & 2 deletions handwritten/firestore/dev/src/write-batch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ export class WriteBatch implements firestore.WriteBatch {
SPAN_NAME_BATCH_COMMIT,
async () => {
// Capture the error stack to preserve stack tracing across async calls.
const stack = Error().stack!;
const callsiteError = Error();

// Commits should also be retried when they fail with status code ABORTED.
const retryCodes = [StatusCode.ABORTED, ...getRetryCodes('commit')];
Expand All @@ -597,7 +597,7 @@ export class WriteBatch implements firestore.WriteBatch {
);
})
.catch(err => {
throw wrapError(err, stack);
throw wrapError(err, callsiteError.stack!);
});
},
{
Expand Down
Loading