Skip to content

feat(hono): Add warning in Bun for double init#21195

Merged
s1gr1d merged 3 commits into
developfrom
sig/hono-bun-double-instrument-warn
May 28, 2026
Merged

feat(hono): Add warning in Bun for double init#21195
s1gr1d merged 3 commits into
developfrom
sig/hono-bun-double-instrument-warn

Conversation

@s1gr1d
Copy link
Copy Markdown
Member

@s1gr1d s1gr1d commented May 27, 2026

Reference #21176

@s1gr1d s1gr1d requested a review from a team as a code owner May 27, 2026 13:47
@s1gr1d s1gr1d requested review from mydea and nicohrubec and removed request for a team May 27, 2026 13:47
Copy link
Copy Markdown
Member

@nicohrubec nicohrubec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. should we do this for cloudflare as well?

expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Sentry is already initialized'));
});

it('warns that initialization should only happen through the sentry() middleware', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: should this test and the one before maybe be merged? i.e. just have one "warns if sentry is already initialized" and then check both logs or just check that the spy is being called

Comment thread packages/hono/src/bun/sdk.ts Outdated
export function init(options: HonoBunOptions): Client | undefined {
const existingClient = getClient();
if (existingClient) {
debug.warn(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe do a console.warn here (wrapped in consoleSandbox)? 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I would probably not return early here but still run init again, second init should take precedence I suppose in such a scenario...? (as much as it can, it will be kind of buggy for sure either way)

@s1gr1d
Copy link
Copy Markdown
Member Author

s1gr1d commented May 28, 2026

Applied your suggestions, they all make sense :)

@s1gr1d s1gr1d merged commit b2aa011 into develop May 28, 2026
48 checks passed
@s1gr1d s1gr1d deleted the sig/hono-bun-double-instrument-warn branch May 28, 2026 08:32
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.

3 participants