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

Fix missing websiteId issue #2946

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

jjjhill
Copy link

@jjjhill jjjhill commented Sep 5, 2024

No description provided.

Copy link

vercel bot commented Sep 5, 2024

Someone is attempting to deploy a commit to the umami-software Team on Vercel.

A member of the Team first needs to authorize it.

left inner join seems more appropriate
@mikecao
Copy link
Collaborator

mikecao commented Oct 25, 2024

Isn't the missing website id issue related to sending events? So requests to /api/send. Not sure how this is related.

@Benjamin-Dobell
Copy link

Benjamin-Dobell commented Nov 14, 2024

This won't work on account of left inner join not actually being a thing. The correct PR is left join.

Isn't the missing website id issue related to sending events? So requests to /api/send. Not sure how this is related.

Even when website is included in calls to /send you end up in this state if identify is the first data sent for a particular session ID. The initial /send works just fine and creates a session. Subsequent requests involving the session fail indefinitely.

CleanShot 2024-11-15 at 04 59 34@2x

However, since session is not returned directly, but instead used in a spread, the returned "session" only contains a visitId rather than valid session data.

EDIT: Probably worth noting that logic that avoids throwing shouldn't exist. If you're worried about race conditions you can either re-query or use an upsert i.e. ON CONFLICT DO UPDATE would be more efficient.

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.

3 participants