ky icon indicating copy to clipboard operation
ky copied to clipboard

Expose timings on response

Open guillaumekh opened this issue 3 years ago • 8 comments

got's Response object includes a very useful timingsobject.. It would be neat if KY exposed similar metrics.

It looks like the required events (e.g. request start, TCP handshake, TLS handshake, First Byte…) are exposed by the Resource Timing API and Performance API. Browser support looks pretty good now.

guillaumekh avatar Mar 03 '21 13:03 guillaumekh

Sounds good to me.


It looks like the required events (e.g. request start, TCP handshake, TLS handshake, First Byte…) are exposed by the Resource Timing API and Performance API.

Do you know how we would detect which event comes from a Fetch request?

sindresorhus avatar Mar 03 '21 19:03 sindresorhus

Quick disclaimer, I'm not a fulltime programmer and am not very familiar with either Performance API or Resource Timing API.

It looks like the only way to get the timings is to retrieve the PerformanceResourceTiming object from the window.performance.getEntries() array when the fetch() promise resolves/rejects.

For example:

window.performance.getEntries({
    initiatorType: "fetch",
    name: "someUrl"
}).pop();

which returns

{
    "name": "someUrl",
    "entryType": "resource",
    "startTime": 2545706.055000017,
    "duration": 170.84999999497086,
    "initiatorType": "fetch",
    "nextHopProtocol": "h2",
    "workerStart": 0,
    "redirectStart": 0,
    "redirectEnd": 0,
    "fetchStart": 2545706.055000017,
    "domainLookupStart": 2545706.055000017,
    "domainLookupEnd": 2545706.055000017,
    "connectStart": 2545706.055000017,
    "connectEnd": 2545706.055000017,
    "secureConnectionStart": 2545706.055000017,
    "requestStart": 2545708.5300000035,
    "responseStart": 2545875.620000006,
    "responseEnd": 2545876.905000012,
    "transferSize": 5523,
    "encodedBodySize": 1421,
    "decodedBodySize": 5962,
    "serverTiming": [],
    "workerTiming": []
}

It's not ideal and I'm not sure it will be 100% reliable. What do you think ?

If you're comfortable with it, I'm happy to fill a PR. I was thinking of inserting the logic here before the hooks are run : https://github.com/sindresorhus/ky/blob/d11af8816c0bea0fbca3d844e6a821362de40f5a/index.js#L255-L263

guillaumekh avatar Mar 04 '21 14:03 guillaumekh

No worries, @guillaumekh! They are relatively new APIs and not widely used yet. I do a lot of development in the browser and I'm not very familiar with them either.

As you mentioned, that approach looks unreliable. I'm worried that it would be prone to race conditions. It's kind of ridiculous that a timing measurement API would be so prone to timing related bugs...

Anyway, I'm in favor of this feature if we can reliably associate the timings with a particular Ky request, as opposed to just a recentfetch request that may or may not even be from Ky.

sholladay avatar Mar 05 '21 02:03 sholladay

Maybe we can save the Ky request start time and compare it to the startTime property in the event?

sindresorhus avatar Mar 05 '21 02:03 sindresorhus

Maybe we can save the Ky request start time and compare it to the startTime property in the event?

We can. However, the timestamps won't be exact matches since those are DOMHighResTimeStamps so a tolerance margin will probably be required. I could do a bit of research to figure out a typical tolerance margin but I'm afraid my results would be highly tied to my local setup and ultimately prone to false positives/negatives. Matching the request start time won't completely remove the risk of a false match.

What do you think of adding the feature with an 'experimental' label and a warning on its reliability, and making it optional (e.g. a timings bool in request options)?

It might still provide value for a large segment of users. If the start time matching is added, the only case I can think of where the race condition remains is with multiple requests triggered in very quick succession to the same URL. Maybe that's narrow enough to justify accepting the unreliability of the whole thing.

guillaumekh avatar Mar 05 '21 13:03 guillaumekh

I suspect any attempt to match based on a timestamp, even with a tolerance, is also going to be affected by the Spectre and Meltdown mitigations. Not sure, but I think they are still in effect? https://hackaday.com/2018/01/06/lowering-javascript-timer-resolution-thwarts-meltdown-and-spectre/

sholladay avatar Mar 05 '21 22:03 sholladay

FF currently rounds timestamps to 1 ms increments (source: MDN). It seems likely that other browsers also still have similar mitigations in place.

guillaumekh avatar Mar 08 '21 15:03 guillaumekh

It looks like the only way to get the timings is to retrieve the PerformanceResourceTiming object from the window.performance.getEntries() array when the fetch() promise resolves/rejects.

Relevant spec proposal: https://github.com/w3c/resource-timing/issues/102

septatrix avatar Apr 21 '24 19:04 septatrix