benvinegar/counterscale

Worker throws exception

anurag-roy opened this issue · 5 comments

Hey,

After #26 got merged to master, the worker now throws an exception

Error: The script will never generate a response.

I checked to an earlier commit, and it works fine.

Steps to reproduce:

  1. Deploy from latest commit in master
  2. Visit the /dashboard route, and you should see something like this:
    image

Can be reproduced in dev as well, without deploying.

I think it is due to the code involving invariant, but not sure.

Not directly related to this issue, but I am curious, is there any particular reason why all the async functions/promises are written like this (async IIFE inside a Promise constructor):

async function getVisitorCountByColumn(): Promise<any> {
    const returnPromise = new Promise<any>((resolve, reject) => (async () => {
        const response = await this.query(query);

        if (!response.ok) {
            reject(response.statusText);
        }

        const responseData = await response.json() as AnalyticsQueryResult;
        resolve(responseData.data.map((row) => {
            const key = row[_column] === '' ? '(none)' : row[_column];
            return [key, row['count']];
        }));
    })());
    return returnPromise;
}

Can we not write the above like:

async function getVisitorCountByColumn(): Promise<any> {
    const response = await this.query(query);

    if (!response.ok) {
        throw new Error(response.statusText);
    }

    const responseData = await response.json() as AnalyticsQueryResult;
    return (responseData.data.map((row) => {
        const key = row[_column] === '' ? '(none)' : row[_column];
        return [key, row['count']];
    }));
}

+1 I'm also getting this when trying to run the dev server. I've tried running it on both Window 10 and 11 machines with and without WSL.

This is the error I'm seeing in the terminal:

[REMIX DEV] 37ccc3f6 ready
[wrangler:inf] GET / 200 OK (53ms)
Can't save datapoint: Analytics unavailable
[wrangler:inf] GET /collect 200 OK (16ms)
[wrangler:inf] GET /dashboard 500 Internal Server Error (1026ms)
X [ERROR] Uncaught (in response) Error: The script will never generate a response.

Thanks for reporting – and sorry about this. I obviously did not deploy/verify the change. I think I got swept up by "this is mostly types and some runtime checks" and was too casual about it.

I've hit the revert button so nobody else is affected, and opened #31 and #32 to hopefully minimize this going forward.

is there any particular reason why all the async functions/promises are written like this

The goal was to invoke the query without immediately calling await, so that all the API queries could be parallelized. But you've highlighted that my version doesn't do this correctly. It needs to be changed to invoke this.query, then return the promise.

Closing this now that the issue is resolved, and 24a68f5 addresses the query promise/fetch issue @anurag-roy discovered.

Awesome, sounds good and thanks for resolving things so swiftly! @benvinegar