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

Improve error when Client.Subscribe called on a client that will not work #599

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Sep 18, 2024

Related to #596. If Subscribe was called on a client that didn't have
a Workers bundle configure a nil pointer panic would occur because
subscriptionManager was never initialized.

Here, leave that as a panic since it makes sense to warn a user about an
API misuse that'd undoubtedly lead to more confusion/pain, but improve
the error message so that it's more obvious to the caller why this is a
problem.

Fixes #596.

@brandur brandur requested a review from bgentry September 18, 2024 00:39
CHANGELOG.md Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
client_test.go Outdated Show resolved Hide resolved
…t work

Related to #596. If `Subscribe` was called on a client that didn't have
a `Workers` bundle configure a nil pointer panic would occur because
`subscriptionManager` was never initialized.

Here, leave that as a panic since it makes sense to warn a user about an
API misuse that'd undoubtedly lead to more confusion/pain, but improve
the error message so that it's more obvious to the caller why this is a
problem.

Fixes #596.
@brandur brandur force-pushed the brandur-subscribe-panic branch from 898bfa5 to b1e240c Compare September 18, 2024 15:00
@brandur
Copy link
Contributor Author

brandur commented Sep 18, 2024

ty.

@brandur brandur merged commit b4b8cfe into master Sep 18, 2024
14 checks passed
@brandur brandur deleted the brandur-subscribe-panic branch September 18, 2024 15:04
tigrato pushed a commit to gravitational/river that referenced this pull request Dec 18, 2024
…t work (riverqueue#599)

Related to riverqueue#596. If `Subscribe` was called on a client that didn't have
a `Workers` bundle configure a nil pointer panic would occur because
`subscriptionManager` was never initialized.

Here, leave that as a panic since it makes sense to warn a user about an
API misuse that'd undoubtedly lead to more confusion/pain, but improve
the error message so that it's more obvious to the caller why this is a
problem.

Fixes riverqueue#596.
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.

Unclear panic when using Client.Subscribe() when client is not executing jobs
2 participants