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 31225
Build 48847: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 48846: 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

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

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)

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

post to sentry

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

Unit tests

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

Looks good to me.

swh/graphql/errors/handlers.py
25	Use this instead: if type(error.original_error) not in expected_errors:
swh/graphql/resolvers/scalars.py
54–55	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

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–55	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

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.

Add a centralized error handler
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D8356 (id=30165)

Changes applied before test

Patch application report for D8356 (id=30166)

Changes applied before test

Patch application report for D8356 (id=30167)

Changes applied before test

Patch application report for D8356 (id=30181)

Changes applied before test

Patch application report for D8356 (id=30184)

Changes applied before test

Revision Contents
Changeset List

Diff 30181

config/staging.yml

swh/graphql/errors/init.py

swh/graphql/errors/errors.py

swh/graphql/errors/handlers.py

swh/graphql/resolvers/scalars.py

swh/graphql/server.py

swh/graphql/tests/functional/test_content.py

swh/graphql/tests/functional/utils.py

swh/graphql/tests/unit/errors/test_errors.py

Add a centralized error handlerClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D8356 (id=30165)

Changes applied before test

Patch application report for D8356 (id=30166)

Changes applied before test

Patch application report for D8356 (id=30167)

Changes applied before test

Patch application report for D8356 (id=30181)

Changes applied before test

Patch application report for D8356 (id=30184)

Changes applied before test

Revision ContentsChangeset List

Diff 30181

config/staging.yml

swh/graphql/errors/__init__.py

swh/graphql/errors/errors.py

swh/graphql/errors/handlers.py

swh/graphql/resolvers/scalars.py

swh/graphql/server.py

swh/graphql/tests/functional/test_content.py

swh/graphql/tests/functional/utils.py

swh/graphql/tests/unit/errors/test_errors.py

Add a centralized error handler
ClosedPublic
Actions

Revision Contents
Changeset List

swh/graphql/errors/init.py