Error messages #1165

jssblck · 2023-03-09T22:14:18Z

Overview

During my onduty cycle I've noticed several instances where the error messages reported when running external commands were confusing. This PR is an attempt to make them nicer and more useful.

Additionally, I've added special handling to a couple HTTP error cases:

HTTP 403 errors now indicate they likely are caused by a networking appliance intercepting the request.
HTTP RequestTimeout errors to suggest users ensure they're not uploading a bunch of noise.

Example error text for 403:

The endpoint returned status code 403.

Typically, this status code indicates an authentication problem with the API.
However, FOSSA reports invalid API keys using a different mechanism;
this likely means that some other service on your network intercepted the request
and reported this status code. This might be a proxy or some other network appliance.

    Request: GET https://app.fossa.com/api/<some/example/endpoint>
    Response: 403

This is a networking error.

Networking errors are typically caused by actual network failure or a network appliance
(e.g. a firewall) between the FOSSA CLI and the FOSSA backend.
This means that often such errors are transient, or are caused by local network configuration.

Trying again in a few minutes may resolve this issue.

If this issue persists, please contact FOSSA support at https://support.fossa.com

Example error text for the request timeout:

The request to the FOSSA endpoint took too long to send.

This typically means that the CLI is being asked to send too much data
with the current network speed (for example, uploading large archives),
although this can also be a transient error caused by congested
networking conditions between the CLI and the FOSSA API.

To reduce the likelihood of this error, ensure that only data you really
need FOSSA to scan is being uploaded.

    Request: POST https://app.fossa.com/api/<some/example/endpoint>

If this issue persists, please contact FOSSA support at https://support.fossa.com

Examples for the command errors are below, in testing steps.

Acceptance criteria

Users and FOSSA support staff have more actionable and easy to read error messages, which provide more debugging guidance.

Testing plan

To test exec error rendering in the case of "executable not found", I ran:

; PATH=/home/jess/.ghcup/bin cabal run fossa -- analyze -o ~/projects/scratch/
Up to date
[ INFO] Analyzing cargo project at /home/jess/projects/scratch/
[ERROR] ----------
  An issue occurred

  >>> Details

    Could not generate lock file for cargo manifest: "/home/jess/projects/scratch/Cargo.toml"

  >>> Relevant errors

    Error

      Could not find executable: `cargo`.
      Please ensure `cargo` exists in PATH prior to running fossa.

      If you believe this to be a defect, please report a bug to FOSSA support at https://support.fossa.com

      Traceback:
        - Running command 'cargo'
        - Generating lockfile
        - Dynamic analysis
        - Cargo
        - Project Analysis: CargoProjectType

To test executable failing due to build issues, I made Cargo.toml invalid syntax and ran:

[ERROR] ----------
  An issue occurred

  >>> Details

    Could not generate lock file for cargo manifest: "/home/jess/projects/scratch/Cargo.toml"

  >>> Relevant errors

    Error

      Command execution failed:

          Attempted to run the command 'cargo generate-lockfile'
          inside the working directory '/home/jess/projects/scratch/',
          but failed, because the command exited with code '101'.

          Often, this kind of error is caused by the project not being ready to build;
          please ensure that the project at '/home/jess/projects/scratch/'
          builds successfully before running fossa.

          The logs for the command are listed below.
          They will likely provide guidance on how to resolve this error.
          Command did not write a standard log.

          Command error log:
            error: failed to parse manifest at `/home/jess/projects/scratch/Cargo.toml`

            Caused by:
              could not parse input as TOML

            Caused by:
              TOML parse error at line 1, column 9
                |
              1 | [package
                |         ^
              Unexpected `
              `
              Expected `.` or `]`
              While parsing a Table Header


      If you believe this to be a defect, please report a bug to FOSSA support at https://support.fossa.com

      Traceback:
        - Running command 'cargo'
        - Generating lockfile
        - Dynamic analysis
        - Cargo
        - Project Analysis: CargoProjectType

I don't have a great way to test the two networking errors, as I'm pretty sure they're generally caused by local networking conditions. I think since this is only a change in the documentation rendered in the face of these errors it's safe, but reviewer, if you want me to set up an environment that'd allow me to test these please let me know.

Risks

This change is only to help text rendered in the case of an error, as such I think it's low risk from the standpoint of "successfully executing FOSSA CLI" but medium risk from the standpoint of "not confusing users".

I would love any thoughts folks have on the formatting or content of these messages!

Checklist

I added tests for this PR's change (or explained in the PR description why tests don't make sense).
If this PR introduced a user-visible change, I added documentation into docs/.
If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).

spatten

This looks great! I had one comment about maybe adding a link to documentation in one spot, but nothing blocking

spatten · 2023-03-10T17:54:05Z

src/Control/Carrier/FossaApiClient/Internal/FossaAPIV1.hs

+        , "networking conditions between the CLI and the FOSSA API."
+        , ""
+        , "To reduce the likelihood of this error, ensure that only data you really"
+        , "need FOSSA to scan is being uploaded."


Should we link to docs about inclusion / exclusion here, or other documentation about how to reduce the data you send?

+1 second this

I would like to, but:

At this point without larger refactoring, we're not actually certain that the user is even specifying vendored-dependencies, so linking to it seems like it may be confusing as often as it may be helpful.

We don't have a great way of testing these links to ensure they're durable.

I think we'd ideally link to the documentation like this, but I'd like to consider it out of scope for this PR.

spatten · 2023-03-10T17:56:52Z

src/Effect/Exec.hs

+    details :: Doc AnsiStyle
+    details =
+      vsep
+        [ "Attempted to run the command '" <> prettyCommand cmdFailureCmd <> "'"


I love this! It explains everything so much better and gives much more actionable info

cmboling · 2023-03-10T19:27:49Z

Would this be able to show that the CLI errored out on not using a push only token? I recall receiving this networking error by using a full access token, but I wouldn't say it's a true networking error 🤔 Overall I love this new change! 🚀 ✨ ✨ ✨

Error

      The FOSSA endpoint returned an unexpected status code:

          Request: GET https://app.fossa.com/api/license_scan/signed_url?packageSpec=[RETRACTED]
          Response: 403

      This error is often transient, so trying again in a few minutes may resolve the issue.

      If this issue persists, please report a bug to FOSSA support at https://support.fossa.com/

      In your bug report, please include FOSSA's debug bundle file: fossa.debug.json.gz.

      You can generate debug bundle by using `--debug` flag, for example:
        fossa analyze --debug

      Traceback:
        - Retrieving a signed S3 URL for license scan results of cuda-test
        - Processing vendored dependency
        - Converting fossa-deps to partial API payload
        - fossa-deps
        - fossa-analyze

jssblck · 2023-03-10T20:21:56Z

@cmboling

Would this be able to show that the CLI errored out on not using a push only token? I recall receiving this networking error by using a full access token, but I wouldn't say it's a true networking error 🤔 Overall I love this new change! 🚀 ✨ ✨ ✨

I think the key is that this can't possibly be an issue raised due to an API key error; during the course of this I tested it and we return API key errors very differently.

jssblck added 6 commits March 9, 2023 13:16

Add more detail to HTTP messages

0ee5a93

Pretty exec errors

04baab0

Fix up command rendering

561355d

I was wrong about what 403 means

833d630

Changelog

4cf11bd

make fmt-ci

a480a8f

jssblck marked this pull request as ready for review March 9, 2023 22:17

jssblck requested a review from a team as a code owner March 9, 2023 22:17

jssblck requested a review from spatten March 9, 2023 22:17

spatten approved these changes Mar 10, 2023

View reviewed changes

jssblck merged commit f810e8e into master Mar 11, 2023

jssblck deleted the error-messages branch March 11, 2023 01:05

hubertdeng123 mentioned this pull request Mar 27, 2023

Solve license compliance (again) getsentry/team-ospo#92

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error messages #1165

Error messages #1165

jssblck commented Mar 9, 2023 •

edited

Loading

spatten left a comment

spatten Mar 10, 2023

cmboling Mar 10, 2023

jssblck Mar 10, 2023

spatten Mar 10, 2023

cmboling commented Mar 10, 2023

jssblck commented Mar 10, 2023

Error messages #1165

Error messages #1165

Conversation

jssblck commented Mar 9, 2023 • edited Loading

Overview

Acceptance criteria

Testing plan

Risks

Checklist

spatten left a comment

Choose a reason for hiding this comment

spatten Mar 10, 2023

Choose a reason for hiding this comment

cmboling Mar 10, 2023

Choose a reason for hiding this comment

jssblck Mar 10, 2023

Choose a reason for hiding this comment

spatten Mar 10, 2023

Choose a reason for hiding this comment

cmboling commented Mar 10, 2023

jssblck commented Mar 10, 2023

jssblck commented Mar 9, 2023 •

edited

Loading