thegreenwebfoundation / co2.js

An npm module for accessing the green web API, and estimating the carbon emissions from using digital services
Other
389 stars 49 forks source link

parseOptions validates args it is not supposed to get for perByte(Trace) functions #224

Open alexzurbonsen opened 1 month ago

alexzurbonsen commented 1 month ago

Describe the bug When you call perByte or perByteTrace the function parseOptions also validates args that it cannot get (dataReloadRatio) It logs a lot of stuff which can be consufing in the logs since you get messages that do not apply. Also I am not sure it needs logging at all. After all the values are optional in to begin with. <-- this does not happen for perByteTrace options.

To Reproduce Steps to reproduce the behavior: Call perByte and observe console.logs.

Expected behavior No misleading logs or in this case even better: no logs at all.

fershad commented 3 weeks ago

Hi Alex. Can you please give a code example using both perByte & perByteTrace which returns the logs?

alexzurbonsen commented 3 weeks ago

Hi fershad, of course, at least for perByteTrace. I am not sure anymore, why I also mentioned perByte - I guess that is wrong - as far as I can see the parseOptions call is responsible for the logging.

swd.perByteTrace(
      1000,
      undefined,
      co2jsOptions,
    )

where co2jsOptions is of

type CO2jsOptions = {
  greenHostingFactor: number;
  gridIntensities: {
    device: number | null;
    dataCenter: number | null;
    network: number | null;
  };
};

I simplified CO2jsOptions a little here.

fershad commented 3 weeks ago

Cheers Alex. Yep, I can see that now for perByteTrace.

import { co2 } from '@tgwf/co2'
const estimate = new co2({ model: "swd", version: 4 })
const bytesSent = 1000 * 1000 * 1000; // 1GB expressed in bytes

const options = {
    greenHostingFactor: 0,
    gridIntensities: {
      device: 450,
      dataCenter: 209,
    }
  };

const estimatedCO2 = estimate.perByteTrace(bytesSent, undefined, options);
console.log(estimatedCO2);

Running the code below sees the following console output:

The dataReloadRatio option must be a number. You passed in a undefined.
Falling back to default value.
The firstVisitPercentage option must be a number. You passed in a undefined.
Falling back to default value.
The returnVisitPercentage option must be a number. You passed in a undefined.
Falling back to default value.

{
  co2: 148.2,
  green: false,
  variables: {
    description: 'Below are the variables used to calculate this CO2 estimate.',
    bytes: 1000000000,
    gridIntensity: {
      description: 'The grid intensity (grams per kilowatt-hour) used to calculate this CO2 estimate.',
      device: [Object],
      dataCenter: [Object],
      network: [Object]
    },
    greenHostingFactor: 0
  }
}

Those first three lines are the superfluous warning messages that are being returned, despite being not relevant to the perByteTrace function.

alexzurbonsen commented 2 weeks ago

Yes, that's it. Thank you for looking into it :)

fershad commented 1 week ago

@alexzurbonsen I won't be able to take a proper look at this until early October. If you've got time and capacity between now & then, we'd welcome a PR which I could review when I'm back from leave.