Expose timings on response

[guillaumekh]

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.

[sindresorhus]

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?

[guillaumekh]

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 :

ky/index.js

Lines 255 to 263 in d11af88

	const fn = async () => {
	if (this._options.timeout > maxSafeTimeout) {
	throw new RangeError(`The \`timeout\` option cannot be greater than ${maxSafeTimeout}`);
	}
	await delay(1);
	let response = await this._fetch();
	for (const hook of this._options.hooks.afterResponse) {

[sholladay]

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.

[sindresorhus]

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

[guillaumekh]

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.

[sholladay]

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/

[guillaumekh]

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

[septatrix]

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: w3c/resource-timing#102