1828543 - WPT print-test logs are cluttered with `Loading of pdf failed` which seems to be spammed unconditionally

Status: RESOLVED FIXED

(Testing :: web-platform-tests, task)

Description

[Daniel Holbert [:dholbert]]

In e.g. this log...
https://treeherder.mozilla.org/logviewer?job_id=410888631&repo=autoland&lineNumber=5048
...there are many many copies of this line:

Marionette	WARN	Loading of pdf failed

From a searchfox text-search, it looks like that comes from here:
https://searchfox.org/mozilla-central/rev/31f5847a4494b3646edabbdd7ea39cb88509afe2/remote/marionette/reftest.sys.mjs#794-799

try {
  const pdf = await this.loadPdf(binaryString);
  let pages = this.getPages(pageRanges, url, pdf.numPages);
  return [this.renderPages(pdf, pages), pages.size];
} finally {
  lazy.logger.warn(`Loading of pdf failed`);

If I'm reading that correctly, the finally usage there means we'll log unconditionally, even when there in fact was no failure... Probably we want to do s/finally/catch(e)/ or somesuch?

Comment 1

[Daniel Holbert [:dholbert]]

(I think the return statement example at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch#the_finally-block confirms my recollection of how try...finally works, even in the face of return statements in the try block. So I think we do want catch(e) instead.)

Comment 2

[Emily McDonough [:alaskanemily]]

Attachment: Bug 1828543 - Only log PDF load failure in WPT print-tests when a failure actually occurs

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2d691f2202a
Only log PDF load failure in WPT print-tests when a failure actually occurs r=webdriver-reviewers,jgraham

Comment 4

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/d2d691f2202a