test(google-cloud): Replace error handling using node domain #14908
Conversation
de02754 to
915fe0e
Compare
|
Fixed lint failure. Interesting that I can't get it locally 🤔 . Will look into that. |
AFAIK, there are two types of linter for js/ts code in this repo:
Both are checked in CI |
|
Thank you @Zen-cronic ! Indeed - biome was the one, raising an error about code style. From root of the project Fix added already (squashed and rebased). |
| }), | ||
| ); | ||
|
|
||
| (async () => { |
There was a problem hiding this comment.
Do we even need to make this an IIF?
There was a problem hiding this comment.
not needed per se - it's here to simplify error handling with await to get an exception for both synchronous and asynchronous flows.
can be easily replaced with Promise instead. let me push an update
There was a problem hiding this comment.
Applied a change, please let me know if it looks better now
915fe0e to
01b1e09
Compare
…ry#14769) Replace error handling using deprecated node:domain in google-cloud-serverless tests Fixes getsentry#14769
01b1e09 to
e5c096b
Compare
| @@ -1,4 +1,4 @@ | |||
| import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; | |||
| import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, isThenable } from '@sentry/core'; | |||
There was a problem hiding this comment.
I believe isThenable is not used.
also thank you for this simplification - I've missed the fact that _wrapEventFunction calls callback and no extra Promise waiting needed.
There was a problem hiding this comment.
should I just add a commit to fix biome warning? not sure about squashing at this point though.
There was a problem hiding this comment.
we squash all of our prs - don't worry about clean commit history
There was a problem hiding this comment.
ah ok - thanks for clarification. pushed a fix
|
omg thanks @AbhiPrasad for actually merging this. I had too much on my plate. Sorry for letting this fall through. |
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #14908 --------- Co-authored-by: AbhiPrasad <18689448+AbhiPrasad@users.noreply.github.com> Co-authored-by: Francesco Gringl-Novy <francesco.novy@sentry.io>
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Hi!
I'd like to take an opportunity and make a small contribution to this project and this good first issue to fix issue #14769.
As it turns out -
domainapi is deprecated since really long time and there is no one to one replacement so far.This PR implements a simple error handling to test failures. This is not a one to one replacement of domain based logic - in tests we have a luxury of knowing which failures to anticipate and handle & assert them accordingly.