Uh oh!
There was an error while loading. Please reload this page.
feat(node): Populate event.contexts for Node.js - #5512
Conversation
AbhiPrasad
left a comment
There was a problem hiding this comment.
Can we add unit/integration tests for this? We can do it in a follow up PR.
Uh oh!
There was an error while loading. Please reload this page.
timfish
commented
Aug 2, 2022
I think the Node integration tests are failing because this change introduces an async event processor and this in turn causes the session/event envelopes to be received in a different order. |
The async OS evaluation needs to handle reentrancy properly. If a second events occurs before the async task has completed, |
Uh oh!
There was an error while loading. Please reload this page.
timfish
commented
Aug 2, 2022
Now only the dynamic properties are calculated on every event and everything else is cached. We don't call |
AbhiPrasad
commented
Aug 5, 2022
Hey Tim, any updates on this? Any help you need from our side to investigate or look into stuff? |
AbhiPrasad
commented
Aug 15, 2022
Hopefully #5579 will unblock the test failures for this PR. |
AbhiPrasad
commented
Aug 16, 2022
With #5579 getting merged in, we can rebase this and see if that helps with the test failures! |
AbhiPrasad
commented
Aug 16, 2022
Do we have to revert some of the previous test fixes? |
This comment was marked as outdated.
This comment was marked as outdated.
onurtemizkan
commented
Aug 17, 2022
@timfish, I'm working on it. 👍 |
AbhiPrasad
commented
Aug 18, 2022
Alright #5596 has been merged in! We can try rebasing and trying again here! |
timfish
commented
Aug 21, 2022
I've been away for a few days but will get back onto this on Monday! |
timfish
commented
Aug 22, 2022
Tests are all passing 🎉🎉🎉🎉🎉 Thanks @onurtemizkan for all your hard work fixing this for me! I see there are some new methods for fetching single events. Is it a better idea to use those over my new filtering method? |
onurtemizkan
commented
Aug 22, 2022
Moving filters inside the nock interceptors helped keep the required event / transaction |
AbhiPrasad
commented
Aug 29, 2022
Alright, so I think once we swap |
timfish
commented
Aug 29, 2022
All done! I also replaced |
AbhiPrasad
left a comment
There was a problem hiding this comment.
Based on changes in getsentry/develop#658
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
AbhiPrasad
commented
Aug 29, 2022
Thanks so much for your patience @timfish - and thanks @onurtemizkan for the fixes!!! |
Venipa
commented
Sep 29, 2022
has this been tested on serverless environments? like azure functions? stacktrace: |
AbhiPrasad
commented
Sep 29, 2022
Hey @Venipa could you open a new issue re: your problem? Would be great to get a) your node version, b) the JS runtime that azure functions uses It would be that |
Much of this code has been in use in the Electron SDK for a long time:
https://github.com/getsentry/sentry-electron/blob/master/src/main/context.ts
The addition of memory and cpu info was more recent:
https://github.com/getsentry/sentry-electron/blob/master/src/main/integrations/additional-context.ts
Closes#5500