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

internal/event: avoid reusing canceled context #5304

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

johanbrandhorst
Copy link
Collaborator

Don't error in ReleaseGate if the context used for the original event has been cancelled. Any error in ReleaseGate causes the controller to terminate its startup. If a user was trying to send a request to the Boundary controller as it started up, the context tied to the request would be reused to attempt logging the observation associated with the request after the logging gate was released. This would always fail, as the context associated with the request was canceled.

We now use a new context timeout for events logged after the release gate is released, if the original context was canceled.

Fixes ICU-15809

Don't error in ReleaseGate if the context used for the original
event has been cancelled. Any error in ReleaseGate causes the
controller to terminate its startup. If a user was trying to send
a request to the Boundary controller as it started up, the context
tied to the request would be reused to attempt logging the
observation associated with the request after the logging gate
was released. This would always fail, as the context associated
with the request was canceled.

We now use a new context timeout for events logged after the release
gate is released, if the original context was canceled.
@johanbrandhorst johanbrandhorst added this to the 0.18.x milestone Dec 3, 2024
@johanbrandhorst johanbrandhorst force-pushed the jbrandhorst-fix-startup-http-request-crash branch from bf00927 to 91f8ff8 Compare December 3, 2024 23:47
Copy link
Collaborator

@moduli moduli left a comment

Choose a reason for hiding this comment

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

Ran the failing e2e test and it seems like this has resolved that error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants