Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add warning and collect details of URL errors in sitemap #830

Merged
merged 33 commits into from
Sep 6, 2024

Conversation

amovar18
Copy link
Collaborator

@amovar18 amovar18 commented Sep 3, 2024

Description

This PR aims to collect errors on URL which occur while analysing the sitemap.

Relevant Technical Choices

  • The CLI catches critical errors when sitemap analysis is running.
  • These errors are accumulated, processed and then these URLs are displayed on the CLI-Dashboard.

Testing Instructions

  • Clone this branch.
  • In the terminal, run npm i and run npm run build:cli:dashboard.
  • Use a CSV or a sitemap where there are URLs with errors.
  • After analysis of the sitemap, open the dashboard, you'll notice those URLs listed under URLs with errors.

Example:
CSV File: psat-csv-test.csv
Run npm run cli -- -f ~/Downloads/psat-csv-test.csv

Additional Information:

Screenshot/Screencast


Checklist

  • I have thoroughly tested this code to the best of my abilities.
  • I have reviewed the code myself before requesting a review.
  • This code is covered by unit tests to verify that it works as intended.
  • The QA of this PR is done by a member of the QA team (to be checked by QA).

Fixes #818

@amovar18 amovar18 self-assigned this Sep 3, 2024
@amovar18 amovar18 added the CLI label Sep 3, 2024
@pavanpatil1
Copy link
Collaborator

pavanpatil1 commented Sep 3, 2024

  • The icons for the URLs with issues are not displaying correctly.
Screenshot 2024-09-03 at 2 40 31 PM
  • If a URL from the CSV or sitemap fails, the CLI results currently do not display a message. We should add a message that says, 'Some URLs encountered issues and were not executed properly. You can find a details on dashbord' when any URLs fail during execution
Screenshot 2024-09-03 at 3 04 11 PM
  • This is the description of the error, so the column name should be 'Error Description'.
Screenshot 2024-09-03 at 5 23 24 PM

@amovar18 amovar18 marked this pull request as ready for review September 6, 2024 05:44
@amovar18 amovar18 added this to the v1.0 milestone Sep 6, 2024
urlsToProcess,
cookieAnalysisAndFetchedResourceData,
technologyAnalysisData
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Collaborator

@mohdsayed mohdsayed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a few comments to address.

@mohdsayed mohdsayed merged commit bd1c66e into develop Sep 6, 2024
6 checks passed
@mohdsayed mohdsayed deleted the feat/adding-warning-for-server-error branch September 6, 2024 13:45
@maitreyie-chavan maitreyie-chavan modified the milestones: v1.0, v0.12 Oct 16, 2024
amovar18 added a commit that referenced this pull request Dec 5, 2024
* adding server error check

* Throw error properly without printing the whole stack trace.

* Add failure cross when analysis fails.
Continue analysing in sitemap if single url is invalid.

* Add support to collect all erroredOutUrls.

* Move getSiteReport to utils package.

* Add Urls with issues to cli dashboard.

* Add support for stackTrace in error collection.

* Add URL with issue icon.

* Add error stack trace properly.

* Fix failing tests

* Fix cli e2e

* Fix cli e2e and update the icon size.

* Test cli e2e test passing.

* Remove rmsync.

* Change the url for testing.

* Remove debug.

* Skip CLI E2E tests.

* Change description to error description.

* Add a console.log message to indicate failing urls.

* Add method to generate error logs.

* Prettify stack trace.
Add error.log in the stack trace.

* Add feedback for the urls.

* Fix bugs.

* Fix name or urls.

* Fix condition.

* Fix QA feedbacks.

* Fix url in sidebar.

* use url instead of _url

* Use shorthand and use index.

* Refactor code.

* Move error logs in common and download errorlog when using -o.

* Address minor code feedbacks.

---------

Co-authored-by: Fellyph Cintra <fellyph.cintra@gmail.com>
@mohdsayed mohdsayed mentioned this pull request Dec 5, 2024
amovar18 added a commit that referenced this pull request Dec 12, 2024
* adding server error check

* Throw error properly without printing the whole stack trace.

* Add failure cross when analysis fails.
Continue analysing in sitemap if single url is invalid.

* Add support to collect all erroredOutUrls.

* Move getSiteReport to utils package.

* Add Urls with issues to cli dashboard.

* Add support for stackTrace in error collection.

* Add URL with issue icon.

* Add error stack trace properly.

* Fix failing tests

* Fix cli e2e

* Fix cli e2e and update the icon size.

* Test cli e2e test passing.

* Remove rmsync.

* Change the url for testing.

* Remove debug.

* Skip CLI E2E tests.

* Change description to error description.

* Add a console.log message to indicate failing urls.

* Add method to generate error logs.

* Prettify stack trace.
Add error.log in the stack trace.

* Add feedback for the urls.

* Fix bugs.

* Fix name or urls.

* Fix condition.

* Fix QA feedbacks.

* Fix url in sidebar.

* use url instead of _url

* Use shorthand and use index.

* Refactor code.

* Move error logs in common and download errorlog when using -o.

* Address minor code feedbacks.

---------

Co-authored-by: Fellyph Cintra <fellyph.cintra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants