Closed Bug 1782421 Opened 4 years ago Closed 4 years ago

output-file should accept and log Result `Err` returns from formatting logic instead of unwrapping them

Categories

(Webtools :: Searchfox, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: asuth, Assigned: asuth)

References

Details

Attachments

(1 file)

In bug 1781399 we're seeing output-file abort, presumably because the tokenizer is unhappy about an unpaired string because the "rust" tree checked in some test cases with intentionally invalid files. But it's hard to know exactly because Result::unwrap's output doesn't seem to be making it to the ".par" files generated by parallel.

Our format logic already has a return type of "Result" in the first case, so we can easily migrate to logging the error, but the meta-issues raised in bug 1782375 of how not causing an immediate cascade of failures can let functionality break over time stands. But as arrived at in that bug, the reality is that:

  1. If you liked it then you shoulda put a check on it. Our check mechanism quite explicitly knows how to validate snippets of HTML output so we no longer need to depend on nothing ever having problems.
  2. We don't have the maintainer bandwidth to operate in a fail-fast mode. config4 has been broken for a solid week.

In this expected failure mode, because we flush data to a writer as we go, it's quite likely there's already partial data present in the output, and this potentially does provide useful information about the lead-up. Because the output should leave the HTML parser in a usable state outside of tags, we can probably just emit the failure information as HTML and call it a day. In the future we can emit information about the failure to a more centralized journal. That could potentially be done via a move to running things under searchfox-tool with this providing the parallelism, which is a lot easier than trying to gather data from a bunch of parallel processes and their ".par" file output.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Note that this did not actually fix bug 1781399 so whatever is going on there is different. We probably want to revisit how we're using parallel and any rust configuration that would be preventing us from receiving unwrap() panics.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: