Skip to content

Conversation

@r0x0d
Copy link
Contributor

@r0x0d r0x0d commented Oct 7, 2025

Summary

This bug got introduced in when merging dc29288 (#4648). The problem was that the code was trying to load a session id from the database that does not exist.

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

Testing

Related Issues

Relates to #ISSUE_ID
Discussion: LINK (if any)

Screenshots/Demos (for UX changes)

Before:

After:

Email:

This bug got introduced in when merging  dc29288 (block#4648).  The problem was that the code was trying to load a session id from the database that does not exist.

Signed-off-by: Rodolfo Olivieri <[email protected]>
@r0x0d r0x0d force-pushed the fix-empty-sessions branch from 7425d45 to ab8e0f6 Compare October 7, 2025 17:16
@r0x0d r0x0d mentioned this pull request Oct 7, 2025
@DOsinga
Copy link
Collaborator

DOsinga commented Oct 8, 2025

What exactly are you trying to fix here? if the session doesn't exist in the database and you try to resume it, I think we should exit. We can make the error reporting better if that is the case, but creating a new session doesn't seem like the right fix

@DOsinga DOsinga self-requested a review October 8, 2025 19:41
@r0x0d
Copy link
Contributor Author

r0x0d commented Oct 9, 2025

What exactly are you trying to fix here? if the session doesn't exist in the database and you try to resume it, I think we should exit. We can make the error reporting better if that is the case, but creating a new session doesn't seem like the right fix

Hi, @DOsinga. Sorry for the lack of explanation.

I posted the error I got here: #4648 (comment). Since #4648 got merged, I couldn't run goose properly. My patch fixed it, but not sure if that's the correct approach.

Even if I clean up the session's folder, goose can't really start up. I can test in a new environment (without goose being present previously) and post here tomorrow.

@DOsinga
Copy link
Collaborator

DOsinga commented Oct 12, 2025

thanks for the context. so are you seeing this every time you start goose? that's pretty bad if so. my concern is a little bit that if we have a sesion id and we can't find that session id, how did we get that session id? if the user provided it, we should error out. it really doesn't exist. but if we somehow think we created one and now have one that doesn't exist, we should try to fix that code path. can you print the session id that it fails with? and see if you can get a stack trace out where it came from?

@r0x0d
Copy link
Contributor Author

r0x0d commented Oct 13, 2025

thanks for the context. so are you seeing this every time you start goose? that's pretty bad if so. my concern is a little bit that if we have a sesion id and we can't find that session id, how did we get that session id? if the user provided it, we should error out. it really doesn't exist. but if we somehow think we created one and now have one that doesn't exist, we should try to fix that code path. can you print the session id that it fails with? and see if you can get a stack trace out where it came from?

Yeah, totally. That's a very good idea. Let me put this in a draft and try to debug more.

@r0x0d r0x0d marked this pull request as draft October 13, 2025 11:29
@DOsinga
Copy link
Collaborator

DOsinga commented Oct 17, 2025

I think this can be closed now since we solved it in a different way!

@r0x0d
Copy link
Contributor Author

r0x0d commented Oct 17, 2025

I think this can be closed now since we solved it in a different way!

oh, cool! let me close then.

@r0x0d r0x0d closed this Oct 17, 2025
@r0x0d r0x0d deleted the fix-empty-sessions branch October 17, 2025 20:40
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