fix(bun): Ensure instrumentation of Bun.serve survives a server reload
#15148
Conversation
| import { getDynamicSamplingContextFromSpan, setCurrentClient, spanIsSampled, spanToJSON } from '@sentry/core'; | ||
|
|
||
| import { BunClient } from '../../src/client'; | ||
| import { instrumentBunServe } from '../../src/integrations/bunserver'; | ||
| import { getDefaultBunClientOptions } from '../helpers'; | ||
|
|
||
| // Fun fact: Bun = 2 21 14 :) | ||
| const DEFAULT_PORT = 22114; | ||
|
|
||
| describe('Bun Serve Integration', () => { |
There was a problem hiding this comment.
I discovered that none of the tests in this file were actually working — all of the client.on('spanEnd', ...) were never being called and since all the expects were inside of them nothing failed.
I have restructured the tests to make sure that if the events ever stop firing again in the future, the tests will fail, then fixed all the tests.
| describe('Bun Serve Integration', () => { | ||
| let client: BunClient; | ||
| // Fun fact: Bun = 2 21 14 :) | ||
| let port: number = 22114; |
There was a problem hiding this comment.
We can't reuse the port on each start since server.stop happens in the background and tests end up using each other's servers.
| const initOptions = { | ||
| dsn: 'https://[email protected]/0000000', | ||
| tracesSampleRate: 1, | ||
| }; |
There was a problem hiding this comment.
It turns out that the SDK initialised in here is effecting the SDK being used by bunserver.test.ts above. I am not familiar enough with the Sentry codebase to know how to properly reset after these tests but for sure something is missing here as the tests are not properly isolated.
I've had to add tracesSampleRate: 1 for now to ensure that the SDK being reused doesn't totally break the other tests which depend on tracing happening for every single call.
nathankleyn
force-pushed
the
15144-fix-bun-instrumentation-disappearing-after-reload
branch
from
16b41ad to
8fb5c8f
Compare
If `#reload` is called on an instance of `Bun.serve`, the Sentry intrumentation doesn't surive. This is because the Bun instrumentation works by using `Proxy` on the call to `Bun.serve`, which isn't called for a reload. We can't wrap the serve created by calling `Bun.serve` with a `Proxy` as Bun seems to do some internal checks using `instanceof` which break if the instance is now reporting itself as a `ProxyObject`. This fixes getsentry#15144.
nathankleyn
force-pushed
the
15144-fix-bun-instrumentation-disappearing-after-reload
branch
from
8fb5c8f to
f946c81
Compare
|
Hey @nathankleyn thanks for the PR! Assigning myself so that I can help get this merged in. |
AbhiPrasad
changed the title
Bun.serve survives a server reload
…oad (#15148) If `#reload` is called on an instance of `Bun.serve`, the Sentry intrumentation doesn't surive. This is because the Bun instrumentation works by using `Proxy` on the call to `Bun.serve`, which isn't called for a reload. We can't wrap the serve created by calling `Bun.serve` with a `Proxy` as Bun seems to do some internal checks using `instanceof` which break if the instance is now reporting itself as a `ProxyObject`.
This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #15148 --------- Co-authored-by: AbhiPrasad <[email protected]> Co-authored-by: Abhijeet Prasad <[email protected]>
nathankleyn
deleted the
15144-fix-bun-instrumentation-disappearing-after-reload
branch
If
#reloadis called on an instance ofBun.serve, the Sentry intrumentation doesn't surive. This is because the Bun instrumentation works by usingProxyon the call toBun.serve, which isn't called for a reload.We can't wrap the serve created by calling
Bun.servewith aProxyas Bun seems to do some internal checks usinginstanceofwhich break if the instance is now reporting itself as aProxyObject.This fixes #15144.
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).