Skip to content

Fix crash of the stats page with goals#91

Open
edarchis wants to merge 1 commit intovinceanalytics:mainfrom
edarchis:main
Open

Fix crash of the stats page with goals#91
edarchis wants to merge 1 commit intovinceanalytics:mainfrom
edarchis:main

Conversation

@edarchis
Copy link

@edarchis edarchis commented Nov 6, 2025

See issue #77 for details but in a nutshell:

  • create a goal "toto"
  • trigger the event from your web page (note that the documentation mixes plausible-event-name and vince-event-name but only the plausible one works)
  • go to the stats page => page blows up with an exception e.length on undefined e.

That's because the event doesn't have a .name, only .event. I supported both in list.js and shielded trimUrl that was starting with url.length...

I didn't find a previous PR with updates to the Javascript and since the JS artefacts seem to be manually generated, I included them in a separate commit. Feel free to exclude it, cherry pick the rest and regenerate it yourself, or ask me to remove it if necessary. I know that I didn't (intentionally) include any malicious code in there but I wouldn't trust a stranger to include minified JS code like that in my projects. Maybe adding a Github action would be useful ?

I also removed some deprecated .substr() in the same file.

See issue vinceanalytics#77 for details but showing stats for goals crashed the page. The issue was that the goal events don't have a .name, only a .event. And trimUrl started with .length...
This commit fixes the crash but the link for the details is still pointing to .../?filters=((is,goal,(null)))
The trimUrl is also now shielded and logging about these errors.
Copy link
Collaborator

@gernest gernest left a comment

Choose a reason for hiding this comment

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

@edarchis thanks for taking a stab at this.

I didn't find a previous PR with updates to the Javascript and since the JS artefacts seem to be manually generated, I included them in a separate commit

No problem, can you please remove the generated artefact and only leave the js changes so I can review the code ? I will help with updating the generated js separately when releasing the fix.

That's because the event doesn't have a .name, only .event. I supported both in list.js and shielded trimUrl that was starting with url.length...

Can you please expand what you mean by this? Does the event come from the backend? We probably need to know if it is backend of frontend issue.

I also removed some deprecated .substr() in the same file

awesome, however it will be nice if only changes fixing the issue be included here, you can always send unreleated changes in a separate PR.

@edarchis
Copy link
Author

edarchis commented Nov 7, 2025

Sure, I updated my branch to include only the minimal JS fix. You should be able to review just that.

The .substr probably isn't worth a PR of its own, maybe one day if I feel like going the whole codebase to chase all of them. It was just my IDE warning about it.

Regarding the issue itself, when I visit the page, I have a first event:

{"n":"pageview","u":"https://www.xxx.com/","d":"xxx.com","r":null}

Then when I click the button with the goal:

{"n":"Homepage_button","u":"https://www.xxx.com/","d":"xxx.com","r":null,"p":{"url":"https://app.xxx.com/secure/register"}}

Seems fair.

The api request on the stats page is then:

https://vince.xxx.com/api/stats/xxx.com/conversions/?period=day&date=2025-11-07&filters=[]&with_imported=true&limit=9

yielding:

{"results":[{"conversion_rate":6,"event":"Homepage_button","events":0,"visitors":1}]}

The issue might be that the backend doesn't return a name in the events/conversions but I'm not sure and it wasn't immediately obvious to me when reading the Go code.

@gernest, some opinion on this would help avoid wasting time in the wrong direction. Thanks.

@edarchis edarchis requested a review from gernest November 7, 2025 14:49
@gernest
Copy link
Collaborator

gernest commented Nov 7, 2025

Thanks , I will take a look tomorrow. I want to see if we can resolve this on the backend first before accepting this patch, since the issue is very clear here.

Goals can be tied to event name or path, still need to investigate further in case this can be triggered by path as well which will mean something is broken with the backend.

@l-you
Copy link

l-you commented Jan 5, 2026

Any update on this?

@edarchis
Copy link
Author

edarchis commented Jan 6, 2026

There is indeed a backend issue over here but I didn't have time to dig into it and try to make the proper fix.
Still, the frontend shouldn't crash the whole page when we trigger this situation. I deployed this patch on my own server because it was a blocking issue. Maybe I'll take time to check the backend at some point.

@l-you
Copy link

l-you commented Jan 6, 2026

The same.

The issue could be prevented by utilizing Typescript for the frontend. Also, I would rewrite the API usage in the dashboard to the graphql via gqlgen on the server side and graphql-code-generator client-preset in the dashboard. This would completely prevent such type of errors.

I would work on such an update myself if @gernest would merge it.

@brunohaid
Copy link

Same issue here - @gernest can you please merge und publish a new release? Thanks a lot!

@gernest
Copy link
Collaborator

gernest commented Jan 15, 2026

@brunohaid

I just can't find time to look into this. From analysis I highly think it is a backend issue. I can't just merge this until I rule out that it is a frontend issue (which is highly unlikely) as I already explained how goals are handled.

This PR is a very good starting point and a work around, however I can't just merge it without proper review since I will be the one maintaining it.

@l-you
Copy link

l-you commented Jan 26, 2026

@edarchis Even after I forked (https://github.com/l-you/vince) and fixed the issue, the server was still randomly crashing. So, I tired of it and created my own analytics written in GO. With strict GraphQL APIs and Typescript to avoid type errors during runtime. I would like to have some feedback about it: https://github.com/RevoTale/lovely-eye

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.

4 participants