Skip to content
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.

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

fix: guard against apm-clients encoding/truncation of trace objects throwing #4359

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 10, 2024

We have observed an exception from 'truncate.transaction()' twice,
to my knowledge. I have been unable to reproduce it so far.
A possible side-effect of client._encode throwing in the
apm-client is that it effectively gets wedged ('corked()').

Refs: #1966

…hrowing

We have observed an exception from 'truncate.transaction()' twice,
to my knowledge. I have been unable to reproduce it so far.
A possible side-effect of client._encode throwing in the
apm-client is that it effectively gets wedged ('corked()').

Refs: #1966
@trentm trentm requested a review from david-luna December 10, 2024 06:31
@trentm trentm self-assigned this Dec 10, 2024
@trentm
Copy link
Member Author

trentm commented Dec 10, 2024

This guard is, I believe, the right thing to do. However it is slightly frustrating because I've not been able to reproduce a case where truncate.transaction(...) hits the exception we've observed.

The second instance I've seen in the wild was slightly different in that it threw on node_modules/breadth-filter/index.js:40, instead of line 35 (as in the stacktrace in #1966).

This is a wild guess, but in both instances the attribute name (created_at, person_type) suggests a possible link to usage of sequelize. I do see some Object.defineProperty handling in sequelize that could possibly be related, but I've not managed to reproduce anything yet.

david-luna
david-luna previously approved these changes Dec 10, 2024
@david-luna david-luna mentioned this pull request Dec 10, 2024
@trentm trentm merged commit 12d6d66 into main Dec 10, 2024
18 checks passed
@trentm trentm deleted the trentm/guard-against-trans-encode-throw branch December 10, 2024 18:08
trentm added a commit that referenced this pull request Jan 7, 2025
…hrowing (#4359)

We have observed an exception from 'truncate.transaction()' twice,
to my knowledge. I have been unable to reproduce it so far.
A possible side-effect of client._encode throwing in the
apm-client is that it effectively gets wedged ('corked()').

Refs: #1966
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants