Page MenuHomeSoftware Heritage
Differential D8356

Add a centralized error handler
ClosedPublic
Actions

Authored by jayeshv on Aug 31 2022, 12:00 PM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T4300: GraphQL: Collect code metric and add logging
Commits
rDGQLcae785639016: Add a centralized error handler
Summary

return the stack trace when the debug is set to True.
Log all the errors and send only the unknown ones to sentry.

Improve error messages.

Related to T4300

Diff Detail

Repository
rDGQL GraphQL API
Branch
sentry
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31209
Build 48817: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 48816: arc lint + arc unit

Event Timeline

jayeshv created this revision.Aug 31 2022, 12:00 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 31 2022, 12:00 PM
swh-public-ci added a comment.Aug 31 2022, 12:02 PM

Build is green

Patch application report for D8356 (id=30165)

Rebasing onto 4478a533a9...

Current branch diff-target is up to date.
Changes applied before test
commit fd5e172270f81b04b0171ce03f7b0d3fb2f053f4
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 31 11:40:24 2022 +0200

    Add a centralized error handler
    
    return the stack trace when the debug is set to True.
    Log all the errors and send only the unknown ones to  sentry.
    
    Related to T4300

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/140/ for more details.

Harbormaster completed remote builds in B31209: Diff 30165.Aug 31 2022, 12:02 PM
jayeshv requested review of this revision.Aug 31 2022, 12:02 PM
jayeshv added a subscriber: ardumont.Aug 31 2022, 12:03 PM
jayeshv added inline comments.
swh/graphql/errors/handlers.py
24

@ardumont I would like to send only these errors to sentry, I hope just a raise here will do the trick

jayeshv updated this revision to Diff 30166.Aug 31 2022, 1:46 PM

doc improvements

jayeshv retitled this revision from [WIP] Add a centralized error handler to Add a centralized error handler.Aug 31 2022, 1:47 PM
jayeshv edited the summary of this revision. (Show Details)
swh-public-ci added a comment.Aug 31 2022, 1:48 PM

Build is green

Patch application report for D8356 (id=30166)

Rebasing onto 4478a533a9...

Current branch diff-target is up to date.
Changes applied before test
commit 7c26384ac4e99aad7ada6fe7693776b2284644b1
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 31 11:40:24 2022 +0200

    Add a centralized error handler
    
    return the stack trace when the debug is set to True.
    Log all the errors and send only the unknown ones to  sentry.
    
    Improve error messages.
    
    Related to T4300

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/141/ for more details.

Harbormaster completed remote builds in B31210: Diff 30166.Aug 31 2022, 1:48 PM
jayeshv updated this revision to Diff 30167.Aug 31 2022, 2:03 PM

post to sentry

swh-public-ci added a comment.Aug 31 2022, 2:05 PM

Build is green

Patch application report for D8356 (id=30167)

Rebasing onto 4478a533a9...

Current branch diff-target is up to date.
Changes applied before test
commit 8c413e26dc33e3184f3439fadf9042a38f7de082
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 31 11:40:24 2022 +0200

    Add a centralized error handler
    
    return the stack trace when the debug is set to True.
    Log all the errors and send only the unknown ones to  sentry.
    
    Improve error messages.
    
    Related to T4300

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/142/ for more details.

Harbormaster completed remote builds in B31211: Diff 30167.Aug 31 2022, 2:05 PM
jayeshv updated this revision to Diff 30181.Aug 31 2022, 3:33 PM

Unit tests

swh-public-ci added a comment.Aug 31 2022, 3:36 PM

Build is green

Patch application report for D8356 (id=30181)

Rebasing onto 4478a533a9...

Current branch diff-target is up to date.
Changes applied before test
commit 1a506e1e20b943960f4c9b1e4196b0a3a37d7321
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 31 11:40:24 2022 +0200

    Add a centralized error handler
    
    return the stack trace when the debug is set to True.
    Log all the errors and send only the unknown ones to  sentry.
    
    Improve error messages.
    
    Related to T4300

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/143/ for more details.

Harbormaster completed remote builds in B31225: Diff 30181.Aug 31 2022, 3:36 PM
anlambert accepted this revision.Aug 31 2022, 3:44 PM
anlambert added a subscriber: anlambert.

Looks good to me.

swh/graphql/errors/handlers.py
23

Use this instead:

if type(error.original_error) not in expected_errors:
swh/graphql/resolvers/scalars.py
54–56

This block is redundant with the one below, I think you can remove it.

This revision is now accepted and ready to land.Aug 31 2022, 3:44 PM
jayeshv updated this revision to Diff 30184.Aug 31 2022, 4:05 PM

address review comments

jayeshv marked 2 inline comments as done.Aug 31 2022, 4:07 PM
jayeshv added inline comments.
swh/graphql/resolvers/scalars.py
54–56

Improved this logic a bit and added a test here
https://forge.softwareheritage.org/D8356#inline-59389:~:text=def%20test_get_content_with_invalid_hash_algorithm(client)%3A

swh-public-ci added a comment.Aug 31 2022, 4:07 PM

Build is green

Patch application report for D8356 (id=30184)

Rebasing onto 4478a533a9...

Current branch diff-target is up to date.
Changes applied before test
commit cae785639016cbf85e8e13c50f03ff89f93482dc
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Aug 31 11:40:24 2022 +0200

    Add a centralized error handler
    
    return the stack trace when the debug is set to True.
    Log all the errors and send only the unknown ones to  sentry.
    
    Improve error messages.
    
    Related to T4300

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/144/ for more details.

Harbormaster completed remote builds in B31228: Diff 30184.Aug 31 2022, 4:07 PM
Closed by commit rDGQLcae785639016: Add a centralized error handler (authored by jayeshv). · Explain WhyAug 31 2022, 4:16 PM
This revision was automatically updated to reflect the committed changes.
jayeshv marked an inline comment as done.
jayeshv added a commit: rDGQLcae785639016: Add a centralized error handler.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
config/
2 lines--
swh/
graphql/
errors/
9 lines100%
Loading...
4 lines100%
Loading...
19 lines33%
Loading...
resolvers/
5 lines91%
Loading...
7 lines41%
Loading...
tests/
functional/
22 lines100%
Loading...

Diff 30165

View Options

config/staging.yml

Loading...
View Options

swh/graphql/errors/__init__.py

Loading...
View Options

swh/graphql/errors/errors.py

Loading...
View Options

swh/graphql/errors/handlers.py

Loading...
View Options

swh/graphql/resolvers/scalars.py

Loading...
View Options

swh/graphql/server.py

Loading...
View Options

swh/graphql/tests/functional/test_error_handling.py

Loading...
About · Developer information · Legal