SEARCH_TIME_EVENT is unusable due to plausible and CORS limitations

[obulat]

Description

vue-plausible only provides client-side integration, and we cannot send events from the server:

https://github.com/moritzsternemann/vue-plausible/blob/56d05277e4082183c700e9f0df4e82b711a00f07/src/nuxt-module.ts#L22-L27

Note: this has never been the problem before because we never recorded server-side events.

On the other hand, on the client side, due to CORS limitations, we cannot get the required headers (["date", "cf-cache-status", "cf-ray"]). Browser limits JS access to only "cache-control", "content-type" and "last-modified"¹.

Reproduction

1. Open a server-side rendered search page on staging: https://staging.openverse.org/search/?q=cat
2. See "Whoops, error". In the console, there is a "t is undefined" error logged.
3. See also that you can search client-side, but the search time event is not logged.

Additional context

The immediate fix for this is to revert the commit in question. This fix is critical because the app's server-side rendered search became unusable.

To correctly send the search time event, we the following two issues need to be opened, with a lower importance label.

Server

• Add ability to manually send analytics POST request from the server #3776

Client

• Expose CORS headers in Django app required for the search time event #3777

Footnotes

1. https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#access-control-expose-headers ↩

[dhruvkb]

Since it's in staging I don't think reverting the commit should be prioritised over applying the recommended fixes that you have already documented.

[obulat]

Since it's in staging I don't think reverting the commit should be prioritised over applying the recommended fixes that you have already documented.

The fixes might take longer than anticipated, and the fix would also depend on the API being deployed, so I think it would be safer to revert.

[sarayourfriend]

This isn't fixed, right? Reopened just in case. We've just reverted the thing causing the issue.

@obulat would a simple solution be to wrap sendCustomEvent with a conditional check on process.client and return a noop when on the server? That would be a stop-gap until we implement, if we want, sending event server side (but I don't see that as a requirement).

We don't need SEARCH_TIME_EVENT for server side requests, because those happen within our VPC, they are fast. The event is meant to capture user experience of those requests, to identify the behaviour of the requests after they leave our network.

[obulat]

This isn't fixed, right? Reopened just in case. We've just reverted the thing causing the issue.

This issue was split into two. I think it's best to fix the underlying issues, and then re-open the original PR to close this issue

@obulat would a simple solution be to wrap sendCustomEvent with a conditional check on process.client and return a noop when on the server? That would be a stop-gap until we implement, if we want, sending event server side (but I don't see that as a requirement).

I guess we would have to wrap $plausible here:

openverse/frontend/src/composables/use-analytics.ts

Line 15 in b4b0cc9

const { $plausible } = useContext()

So, I think we could use something like

const { $plausible } = process.server ? { $plausible: () => {/** no op*/} } : useContext()

We don't need SEARCH_TIME_EVENT for server side requests, because those happen within our VPC, they are fast. The event is meant to capture user experience of those requests, to identify the behaviour of the requests after they leave our network.

I did not know that we mainly want to measure the client side requests, thank you for this context.

[sarayourfriend]

I did not know that we mainly want to measure the client side requests, thank you for this context.

Hopefully that lets us find a "good enough for now" aspect of this, and we can address being able to send events on the server side later on, when we actually need them. It's an important limitation to document, though...

[obulat]

I lowered the priority here because the PR that was breaking the app was reversed.

[obulat]

Moving this to to-do as the blocking issues have been fixed (#3850 and #3811)