Skip to content

codeintel: display LSIF upload-stage errors in the UI

Warren Gifford requested to merge nsc/lsif-upload-error-handling into main

Created by: Strum355

TLDR:

  1. LSIF uploads that don't upload to object store successfully are marked as failed
  2. LSIF upload UI overhauled to display the error to users
  3. This currently does not surface anything for non-multipart (aka small) uploads as the record is deleted on error
  4. Slight UX issue whereby an upload may display as 'Failed' while src-cli is retrying the upload x amount of times

This PR addresses an issue surfaced by a trial customer. The allocated disk space for minio ended up reaching 100% capacity, meaning LSIF uploads could not be stored in minio before processing begins. This was not displayed in the UI, with those uploads being stuck in an "Uploading" state (see screenie # 1).

The src-cli steps in the customers CI did fail as the frontend returned a 500, so this is an issue purely with the upload record in postgres. As such, this PR adds additional error handling to the upload stage to mark an upload as errored instead of leaving it in uploading stage, which would get cleared out by the AbandonedUploadJanitor.

Screenie # 2 shows the uploaded page for a singular failed LSIF upload. It follows the same pattern as for failed index jobs by making use of some frontend magic I didnt search for that takes strings in the shape of "short text:\n * longer text" and making it a dotted list. Screenie # 2 also shows 2 other minor notes:

  1. The uploading stage text now says "Upload started at ..." instead of "Uploaded at ..." to better reflect that its the start time and to avoid the discrepancy between the upload possibly not having been uploaded on failure and what that stage says.
  2. The uploading stage can now be followed by the "Failed at ..." stage. Previous behaviour didn't include that finalizing stage, meaning there was only the "Uploaded at ..." stage.
  3. The failed upload stage now also gets a comfy red danger colour.

Screenie # 3 just shows the failed upload in the uploads list, thats all.

cc @efritz as I handle the errors at the top level of the chain, I'm unsure how I feel about propagating the upload ID up the callstack. Results in a lot of return nil, -1, err. Perhaps you've an idea on how to structure this better.

Screenie # 1

image

Screenie # 2

image

Screenie # 3

image

Merge request reports