cloudflare / speedtest

Component to perform network speed tests against Cloudflare's edge network
https://speed.cloudflare.com
MIT License
459 stars 34 forks source link

Cannot read properties of undefined (reading 'transferSize') #17

Closed bruceharrison1984 closed 1 year ago

bruceharrison1984 commented 1 year ago

I was trying to incorporate this into a small CLI project(not within a browser), but every request appears to fail, but they are actually succeeding in the background, the error message just isn't very good.

I chased it down to this line:

https://github.com/cloudflare/speedtest/blob/25be96439f9bd4ac2dbb9af574cdf1e23243bb24/src/engines/BandwidthEngine/BandwidthEngine.js#L294

For my testing, I simply tried to log the matching entries via:

    var perf = performance.getEntriesByName(url).slice(-1)[0]; // get latest perf timing
    console.log(perf);  // <---- This is always undefined

Closer inspection revealed that the name property of PerformanceResourceTiming being set to a URL object instead of a string. I'm unfamiliar with the Performance API, but the spec seems to state that the name property would be a string rather than an object, but it's not immediately clear to me if that is correct.

https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEntry/name

The following is just a dump of the PerformanceResourceTiming itself, if I dont use getEntriesByName. You can see the entries exist, but they are not being matched via URL for some reason.

Error fetching https://speed.cloudflare.com/__down?bytes=0: TypeError: Cannot read properties of undefined (reading 'transferSize')

// console.log(performance.getEntries().slice(-1)[0]);

PerformanceResourceTiming {
  name: URL {
    href: 'https://speed.cloudflare.com/__down?bytes=0',
    origin: 'https://speed.cloudflare.com',
    protocol: 'https:',
    username: '',
    password: '',
    host: 'speed.cloudflare.com',
    hostname: 'speed.cloudflare.com',
    port: '',
    pathname: '/__down',
    search: '?bytes=0',
    searchParams: URLSearchParams { 'bytes' => '0' },
    hash: ''
  },
  entryType: 'resource',
  startTime: 3013.801200002432,
  duration: -3013.801200002432,
  initiatorType: 'fetch',
  nextHopProtocol: undefined,
  workerStart: 0,
  redirectStart: 0,
  redirectEnd: 0,
  fetchStart: 3013.801200002432,
  domainLookupStart: undefined,
  domainLookupEnd: undefined,
  connectStart: undefined,
  connectEnd: undefined,
  secureConnectionStart: undefined,
  requestStart: 0,
  responseStart: 0,
  responseEnd: 0,
  transferSize: 300,
  encodedBodySize: 0,
  decodedBodySize: 0
}
vasturiano commented 1 year ago

@bruceharrison1984 thanks for reaching out.

The issue that you're experiencing is essentially because the PerformanceResponseTiming api is not really fully implemented in Node.js, even up to the latest version (v20).

The issue of getEntriesByName returning a URL object instead of a string (contrary to the W3C spec) is only the first in a long series of obstacles caused by that API only being partially implemented in node, specifically their HTTP client module: undici.

There is more info on this issue in that repo.

This essentially means that currently this module is not compatible with Node.js environments. It requires a browser environment, where the API spec is fully implemented and conforming to the spec.

Thus, integrating it in a CLI poses quite difficult challenges until the time that node fully supports it.

bruceharrison1984 commented 1 year ago

@vasturiano Thanks for the response. I presumed it was something along those lines but it's nice to hear some verification as well. I'm going to mark this as closed.

jasonz1987 commented 1 year ago

same issues on react native app, so is there a plan to add a react native plugin for this ? maybe, we should replace the browser web api with https://github.com/oblador/react-native-performance

ToshB commented 11 months ago

The issue that you're experiencing is essentially because the PerformanceResponseTiming api is not really fully implemented in Node.js, even up to the latest version (v20).

https://github.com/nodejs/undici/pull/2481 was just merged, which gets us one step closer to getting a working CLI version. Still need to wait for undici to get updated in Node though, but replacing the isomorphic-fetch-polyfill with undici (main) in this library seems to give us node compatibility now.