firefox-devtools / firefox-devtools/bidi-har-export

Incorrect fallback for missing timings

Open
#48 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
10
Forks
3
PR merge metrics
No merged PRs in 30d

Description

According to the discussion on https://bugzilla.mozilla.org/show_bug.cgi?id=2046698, the logic used in this module to compute timings can be incorrect if some timings are missing.

Quoting Jonathan from the thread
> That code in bidi-har-export doesn't look right to me though.
>
> ```
> let last = timings.requestTime;
>
> harEntry.timings.blocked = this._normalizeTiming(timings.dnsStart - last);
>
> last = timings.dnsStart || last;
> harEntry.timings.dns = this._normalizeTiming(
> timings.dnsEnd - timings.dnsStart,
> );
>
> last = timings.connectStart || last;
> harEntry.timings.connect = this._normalizeTiming(timings.connectEnd - last);
>
> last = timings.tlsStart || last;
> harEntry.timings.ssl = this._normalizeTiming(timings.tlsEnd - last);
>
> last = timings.tlsEnd || last;
> harEntry.timings.send = this._normalizeTiming(timings.requestStart - last);
>
> last = timings.requestStart || last;
> harEntry.timings.wait = this._normalizeTiming(timings.responseStart - last);
>
> last = timings.responseStart || last;
> harEntry.timings.receive = this._normalizeTiming(
> timings.responseEnd - last,
> );
> ```
>
> If there was a DNS lookup, but for some reason the connect start is not reported, then shouldn't it fall back to dnsEnd not dnsStart, because the connect time should not include the dns lookup time (only the ssl time is included in the connect time)
>
> If there is no tls start time reported, only a tlsEnd, assuming the connectStart as the start of TLS is surely wrong too, that is saying the whole connect time was TLS.
>
> If there was no DNS lookup and no connectStart reported only a connectEnd, then the connect time star falls back to `timings.requestTime` there by including whatever real blocked time there was in the reported connect time.
>
> So users of the HAR recorder won't get an error, instead they will get incorrect timings in the HAR.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.