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

dekaf: Refresh Read's journal client auth token when it expires #1726

Closed
wants to merge 1 commit into from

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Oct 21, 2024

Description:

This is a follow-up to #1725. I was still seeing some JWT expired errors, this time they looked like this:

WARN serve{...}: dekaf: error=status: Unauthenticated, message: "parsing jwt: token is expired by 12m4.003303197s", details: [], metadata: MetadataMap { headers: {} }

After some more digging, I realized that a sufficiently long-lived session containing a sufficiently long-lived Read could run into this issue, as the Read's journal client is never refreshed. This fixes that.

Note: I did consider whether it would be cleaner to refactor Read to just take a flow_client::Client and wholly manage its own journal client, but it didn't seem better enough to justify the bigger refactor. I'm low conviction on that though, happy to do that refactor if someone else feels more strongly about it.


This change is Reviewable

This is a follow-up to #1725. I was still seeing some JWT expired errors, this time they looked like this:

```
WARN serve{...}: dekaf: error=status: Unauthenticated, message: "parsing jwt: token is expired by 12m4.003303197s", details: [], metadata: MetadataMap { headers: {} }
```

After some more digging, I realized that a sufficiently long-lived session containing a sufficiently long-lived `Read` could run into this issue, as the Read's journal client is never refreshed. This fixes that.
@jshearer jshearer marked this pull request as ready for review October 21, 2024 22:39
@jshearer
Copy link
Contributor Author

Closing as the current behavior of closing the connection on token timeout is actually desired, as it surfaces issues wrt connections closing/opening more frequently

@jshearer jshearer closed this Oct 22, 2024
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.

1 participant