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

Don't crash on background worker connection error #630

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

JelteF
Copy link
Collaborator

@JelteF JelteF commented Feb 26, 2025

If you had an invalid MotherDuck token you would get into a crash-loop,
because the background worker would fail to connect and crash Postgres.

This fixes that by catching all exceptions thrown by C++ functions in
the background worker.

Resolves #622

If you had an invalid MotherDuck token you would get into a crash-loop,
because the background worker would fail to connect and crash Postgres.

This fixes that by catching all exceptions thrown by C++ functions in
the background worker.

Resolves #622
@JelteF JelteF requested a review from mkaruza February 26, 2025 11:26
@JelteF JelteF merged commit 77f1abf into main Feb 26, 2025
5 checks passed
@JelteF JelteF deleted the dont-crash-on-bgworker-error branch February 26, 2025 14:09
JelteF added a commit that referenced this pull request Feb 26, 2025
With #630 I accidentally broke MotherDuck support. I had only tested
that it would not crash with an incorrect MotherDuck token, but I had
not confirmed that a correct one would continue to work correctly. This
fixes that.

The reason that this broke was that we would hold a raw pointer to a
unique_ptr that was already out of scope. This fixes that by creating
the unique_ptr in the correct scope again.
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