fix: isGithubRateLimited can return null #728

ajay-sentry · 2024-07-31T23:43:03Z

Purpose/Motivation

Saw this on my App, hook returning internal server error.

Should hopefully fix these sentry issues

Notes to Reviewer

Anything to note to the team? Any tips on how to review, or where to start?

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov-qa · 2024-07-31T23:52:54Z

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 2263 tests with 2 failed, 2255 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: graphql_api.tests.test_repository.TestFetchRepository
Test name: test_commits_failed_ordering_on_test_results

self = <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>
sql = 'INSERT INTO "reports_test" ("id", "external_id", "created_at", "updated_at", "repoid", "name", "testsuite", "flags_hash", "failure_rate", "commits_where_fail") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s::text[])'
params = ('finally', UUID('8d0f00e8-ef74-495a-b70c-6593a0863b1a'), datetime.datetime(2024, 8, 1, 23, 17, 9, 491736, tzinfo=datetime.timezone.utc), datetime.datetime(2024, 8, 1, 23, 17, 9, 491743, tzinfo=datetime.timezone.utc), 1811, 'coach', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "reports_test_repoid_name_testsuite_flags_hash"
E               DETAIL:  Key (repoid, name, testsuite, flags_hash)=(1811, coach, , ) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: UniqueViolation

The above exception was the direct cause of the following exception:

self = <graphql_api.tests.test_repository.TestFetchRepository testMethod=test_commits_failed_ordering_on_test_results>

    def test_commits_failed_ordering_on_test_results(self) -> None:
        repo = RepositoryFactory(author=self.owner, active=True, private=True)
        test = TestFactory(repository=repo)
        _test_instance_1 = TestInstanceFactory(
            test=test,
            created_at=datetime.datetime.now(),
            repoid=repo.repoid,
            commitid="1",
        )
        _test_instance_2 = TestInstanceFactory(
            test=test,
            created_at=datetime.datetime.now(),
            repoid=repo.repoid,
            commitid="2",
        )
>       test_2 = TestFactory(repository=repo)

graphql_api/tests/test_repository.py:901: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12............/site-packages/factory/base.py:40: in __call__
    return cls.create(**kwargs)
.../local/lib/python3.12............/site-packages/factory/base.py:528: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:117: in _generate
    return super()._generate(strategy, params)
.../local/lib/python3.12............/site-packages/factory/base.py:465: in _generate
    return step.build()
.../local/lib/python3.12.../site-packages/factory/builder.py:262: in build
    instance = self.factory_meta.instantiate(
.../local/lib/python3.12............/site-packages/factory/base.py:317: in instantiate
    return self.factory._create(model, *args, **kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:166: in _create
    return manager.create(*args, **kwargs)
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:658: in create
    obj.save(force_insert=True, using=self.db)
.../local/lib/python3.12.../db/models/base.py:814: in save
    self.save_base(
.../local/lib/python3.12.../db/models/base.py:877: in save_base
    updated = self._save_table(
.../local/lib/python3.12.../db/models/base.py:1020: in _save_table
    results = self._do_insert(
.../local/lib/python3.12.../db/models/base.py:1061: in _do_insert
    return manager._insert(
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:1805: in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
.../local/lib/python3.12.../models/sql/compiler.py:1822: in execute_sql
    cursor.execute(sql, params)
.../local/lib/python3.12.../integrations/django/__init__.py:641: in execute
    result = real_execute(self, sql, params)
.../local/lib/python3.12.../db/backends/utils.py:67: in execute
    return self._execute_with_wrappers(
.../local/lib/python3.12.../db/backends/utils.py:80: in _execute_with_wrappers
    return executor(sql, params, many, context)
.../local/lib/python3.12.../db/backends/utils.py:84: in _execute
    with self.db.wrap_database_errors:
.../local/lib/python3.12.../django/db/utils.py:91: in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>
sql = 'INSERT INTO "reports_test" ("id", "external_id", "created_at", "updated_at", "repoid", "name", "testsuite", "flags_hash", "failure_rate", "commits_where_fail") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s::text[])'
params = ('finally', UUID('8d0f00e8-ef74-495a-b70c-6593a0863b1a'), datetime.datetime(2024, 8, 1, 23, 17, 9, 491736, tzinfo=datetime.timezone.utc), datetime.datetime(2024, 8, 1, 23, 17, 9, 491743, tzinfo=datetime.timezone.utc), 1811, 'coach', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               django.db.utils.IntegrityError: duplicate key value violates unique constraint "reports_test_repoid_name_testsuite_flags_hash"
E               DETAIL:  Key (repoid, name, testsuite, flags_hash)=(1811, coach, , ) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: IntegrityError

Class name: graphql_api.tests.test_repository.TestFetchRepository
Test name: test_fetch_is_github_rate_limited_but_errors

self = <MagicMock name='error' id='140239454257120'>
args = ('Error when checking rate limit',)
kwargs = {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
expected = call('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
actual = call('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
_error_message = <function NonCallableMock.assert_called_with.<locals>._error_message at 0x7f8be0ce4040>
cause = None

    def assert_called_with(self, /, *args, **kwargs):
        """assert that the last call was made with the specified arguments.
    
        Raises an AssertionError if the args and keyword args passed in are
        different to the last call to the mock."""
        if self.call_args is None:
            expected = self._format_mock_call_signature(args, kwargs)
            actual = 'not called.'
            error_message = ('expected call not found.\nExpected: %s\n  Actual: %s'
                    % (expected, actual))
            raise AssertionError(error_message)
    
        def _error_message():
            msg = self._format_mock_failure_message(args, kwargs)
            return msg
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        actual = self._call_matcher(self.call_args)
        if actual != expected:
            cause = expected if isinstance(expected, Exception) else None
>           raise AssertionError(_error_message()) from cause
E           AssertionError: expected call not found.
E           Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E             Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})

.../local/lib/python3.12/unittest/mock.py:944: AssertionError

During handling of the above exception, another exception occurred:

self = <MagicMock name='error' id='140239454257120'>
args = ('Error when checking rate limit',)
kwargs = {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
            raise AssertionError(msg)
>       return self.assert_called_with(*args, **kwargs)
E       AssertionError: expected call not found.
E       Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E         Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E       
E       pytest introspection follows:
E       
E       Kwargs:
E       assert {'extra': {'e...po_id': 1833}} == {'extra': {'e...repo_id': 31}}
E         
E         Differing items:
E         {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 1833}} != {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
E         Use -v to get more diff

.../local/lib/python3.12/unittest/mock.py:956: AssertionError

During handling of the above exception, another exception occurred:

self = <graphql_api.tests.test_repository.TestFetchRepository testMethod=test_fetch_is_github_rate_limited_but_errors>
mock_log_error = <MagicMock name='error' id='140239454257120'>
mock_determine_rate_limit = <MagicMock name='determine_if_entity_is_rate_limited' id='140239469718528'>
mock_determine_redis_key = <MagicMock name='determine_entity_redis_key' id='140239285977696'>

    @patch("shared.rate_limits.determine_entity_redis_key")
    @patch("shared.rate_limits.determine_if_entity_is_rate_limited")
    @patch("logging.Logger.error")
    @override_settings(IS_ENTERPRISE=True, GUEST_ACCESS=False)
    def test_fetch_is_github_rate_limited_but_errors(
        self,
        mock_log_error,
        mock_determine_rate_limit,
        mock_determine_redis_key,
    ):
        repo = RepositoryFactory(
            author=self.owner,
            active=True,
            private=True,
            yaml={"component_management": {}},
        )
    
        mock_determine_redis_key.side_effect = Exception("some random error lol")
        mock_determine_rate_limit.return_value = True
    
        data = self.gql_request(
            query_repository
            % """
                isGithubRateLimited
            """,
            owner=self.owner,
            variables={"name": repo.name},
        )
    
        assert data["me"]["owner"]["repository"]["isGithubRateLimited"] is None
    
>       mock_log_error.assert_called_once_with(
            "Error when checking rate limit",
            extra={
                "repo_id": 31,
                "has_owner": True,
                "exc_info": mock_determine_redis_key.side_effect,
            },
        )
E       AssertionError: expected call not found.
E       Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E         Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E       
E       pytest introspection follows:
E       
E       Kwargs:
E       assert {'extra': {'e...po_id': 1833}} == {'extra': {'e...repo_id': 31}}
E         
E         Differing items:
E         {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 1833}} != {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
E         Use -v to get more diff

graphql_api/tests/test_repository.py:838: AssertionError

codecov-public-qa · 2024-07-31T23:52:58Z

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 2263 tests with 2 failed, 2255 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: graphql_api.tests.test_repository.TestFetchRepository
Test name: test_commits_failed_ordering_on_test_results

self = <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>
sql = 'INSERT INTO "reports_test" ("id", "external_id", "created_at", "updated_at", "repoid", "name", "testsuite", "flags_hash", "failure_rate", "commits_where_fail") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s::text[])'
params = ('finally', UUID('8d0f00e8-ef74-495a-b70c-6593a0863b1a'), datetime.datetime(2024, 8, 1, 23, 17, 9, 491736, tzinfo=datetime.timezone.utc), datetime.datetime(2024, 8, 1, 23, 17, 9, 491743, tzinfo=datetime.timezone.utc), 1811, 'coach', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "reports_test_repoid_name_testsuite_flags_hash"
E               DETAIL:  Key (repoid, name, testsuite, flags_hash)=(1811, coach, , ) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: UniqueViolation

The above exception was the direct cause of the following exception:

self = <graphql_api.tests.test_repository.TestFetchRepository testMethod=test_commits_failed_ordering_on_test_results>

    def test_commits_failed_ordering_on_test_results(self) -> None:
        repo = RepositoryFactory(author=self.owner, active=True, private=True)
        test = TestFactory(repository=repo)
        _test_instance_1 = TestInstanceFactory(
            test=test,
            created_at=datetime.datetime.now(),
            repoid=repo.repoid,
            commitid="1",
        )
        _test_instance_2 = TestInstanceFactory(
            test=test,
            created_at=datetime.datetime.now(),
            repoid=repo.repoid,
            commitid="2",
        )
>       test_2 = TestFactory(repository=repo)

graphql_api/tests/test_repository.py:901: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12............/site-packages/factory/base.py:40: in __call__
    return cls.create(**kwargs)
.../local/lib/python3.12............/site-packages/factory/base.py:528: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:117: in _generate
    return super()._generate(strategy, params)
.../local/lib/python3.12............/site-packages/factory/base.py:465: in _generate
    return step.build()
.../local/lib/python3.12.../site-packages/factory/builder.py:262: in build
    instance = self.factory_meta.instantiate(
.../local/lib/python3.12............/site-packages/factory/base.py:317: in instantiate
    return self.factory._create(model, *args, **kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:166: in _create
    return manager.create(*args, **kwargs)
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:658: in create
    obj.save(force_insert=True, using=self.db)
.../local/lib/python3.12.../db/models/base.py:814: in save
    self.save_base(
.../local/lib/python3.12.../db/models/base.py:877: in save_base
    updated = self._save_table(
.../local/lib/python3.12.../db/models/base.py:1020: in _save_table
    results = self._do_insert(
.../local/lib/python3.12.../db/models/base.py:1061: in _do_insert
    return manager._insert(
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:1805: in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
.../local/lib/python3.12.../models/sql/compiler.py:1822: in execute_sql
    cursor.execute(sql, params)
.../local/lib/python3.12.../integrations/django/__init__.py:641: in execute
    result = real_execute(self, sql, params)
.../local/lib/python3.12.../db/backends/utils.py:67: in execute
    return self._execute_with_wrappers(
.../local/lib/python3.12.../db/backends/utils.py:80: in _execute_with_wrappers
    return executor(sql, params, many, context)
.../local/lib/python3.12.../db/backends/utils.py:84: in _execute
    with self.db.wrap_database_errors:
.../local/lib/python3.12.../django/db/utils.py:91: in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>
sql = 'INSERT INTO "reports_test" ("id", "external_id", "created_at", "updated_at", "repoid", "name", "testsuite", "flags_hash", "failure_rate", "commits_where_fail") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s::text[])'
params = ('finally', UUID('8d0f00e8-ef74-495a-b70c-6593a0863b1a'), datetime.datetime(2024, 8, 1, 23, 17, 9, 491736, tzinfo=datetime.timezone.utc), datetime.datetime(2024, 8, 1, 23, 17, 9, 491743, tzinfo=datetime.timezone.utc), 1811, 'coach', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f8bf47699a0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               django.db.utils.IntegrityError: duplicate key value violates unique constraint "reports_test_repoid_name_testsuite_flags_hash"
E               DETAIL:  Key (repoid, name, testsuite, flags_hash)=(1811, coach, , ) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: IntegrityError

Class name: graphql_api.tests.test_repository.TestFetchRepository
Test name: test_fetch_is_github_rate_limited_but_errors

self = <MagicMock name='error' id='140239454257120'>
args = ('Error when checking rate limit',)
kwargs = {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
expected = call('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
actual = call('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
_error_message = <function NonCallableMock.assert_called_with.<locals>._error_message at 0x7f8be0ce4040>
cause = None

    def assert_called_with(self, /, *args, **kwargs):
        """assert that the last call was made with the specified arguments.
    
        Raises an AssertionError if the args and keyword args passed in are
        different to the last call to the mock."""
        if self.call_args is None:
            expected = self._format_mock_call_signature(args, kwargs)
            actual = 'not called.'
            error_message = ('expected call not found.\nExpected: %s\n  Actual: %s'
                    % (expected, actual))
            raise AssertionError(error_message)
    
        def _error_message():
            msg = self._format_mock_failure_message(args, kwargs)
            return msg
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        actual = self._call_matcher(self.call_args)
        if actual != expected:
            cause = expected if isinstance(expected, Exception) else None
>           raise AssertionError(_error_message()) from cause
E           AssertionError: expected call not found.
E           Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E             Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})

.../local/lib/python3.12/unittest/mock.py:944: AssertionError

During handling of the above exception, another exception occurred:

self = <MagicMock name='error' id='140239454257120'>
args = ('Error when checking rate limit',)
kwargs = {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
            raise AssertionError(msg)
>       return self.assert_called_with(*args, **kwargs)
E       AssertionError: expected call not found.
E       Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E         Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E       
E       pytest introspection follows:
E       
E       Kwargs:
E       assert {'extra': {'e...po_id': 1833}} == {'extra': {'e...repo_id': 31}}
E         
E         Differing items:
E         {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 1833}} != {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
E         Use -v to get more diff

.../local/lib/python3.12/unittest/mock.py:956: AssertionError

During handling of the above exception, another exception occurred:

self = <graphql_api.tests.test_repository.TestFetchRepository testMethod=test_fetch_is_github_rate_limited_but_errors>
mock_log_error = <MagicMock name='error' id='140239454257120'>
mock_determine_rate_limit = <MagicMock name='determine_if_entity_is_rate_limited' id='140239469718528'>
mock_determine_redis_key = <MagicMock name='determine_entity_redis_key' id='140239285977696'>

    @patch("shared.rate_limits.determine_entity_redis_key")
    @patch("shared.rate_limits.determine_if_entity_is_rate_limited")
    @patch("logging.Logger.error")
    @override_settings(IS_ENTERPRISE=True, GUEST_ACCESS=False)
    def test_fetch_is_github_rate_limited_but_errors(
        self,
        mock_log_error,
        mock_determine_rate_limit,
        mock_determine_redis_key,
    ):
        repo = RepositoryFactory(
            author=self.owner,
            active=True,
            private=True,
            yaml={"component_management": {}},
        )
    
        mock_determine_redis_key.side_effect = Exception("some random error lol")
        mock_determine_rate_limit.return_value = True
    
        data = self.gql_request(
            query_repository
            % """
                isGithubRateLimited
            """,
            owner=self.owner,
            variables={"name": repo.name},
        )
    
        assert data["me"]["owner"]["repository"]["isGithubRateLimited"] is None
    
>       mock_log_error.assert_called_once_with(
            "Error when checking rate limit",
            extra={
                "repo_id": 31,
                "has_owner": True,
                "exc_info": mock_determine_redis_key.side_effect,
            },
        )
E       AssertionError: expected call not found.
E       Expected: error('Error when checking rate limit', extra={'repo_id': 31, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E         Actual: error('Error when checking rate limit', extra={'repo_id': 1833, 'has_owner': True, 'exc_info': Exception('some random error lol')})
E       
E       pytest introspection follows:
E       
E       Kwargs:
E       assert {'extra': {'e...po_id': 1833}} == {'extra': {'e...repo_id': 31}}
E         
E         Differing items:
E         {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 1833}} != {'extra': {'exc_info': Exception('some random error lol'), 'has_owner': True, 'repo_id': 31}}
E         Use -v to get more diff

graphql_api/tests/test_repository.py:838: AssertionError

codecov · 2024-07-31T23:58:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.04%. Comparing base (183451b) to head (3c05e9c).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@               Coverage Diff                @@
##               main       #728        +/-   ##
================================================
+ Coverage   96.02000   96.04000   +0.02000     
================================================
  Files           814        814                
  Lines         18407      18496        +89     
================================================
+ Hits          17676      17765        +89     
  Misses          731        731

Flag	Coverage Δ
unit	`91.74% <100.00%> (+0.01%)`	⬆️
unit-latest-uploader	`91.74% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

spalmurray-codecov · 2024-08-01T13:22:08Z

@ajay-sentry I was getting this on my local environment yesterday. I spoke with Rohit and it sounds like this should never return null. If it is, there is a problem elsewhere. I'll send u our thread on Slack. That said, I'm pretty concerned about this because the symptom on my local app was gazebo wouldn't load at all. Hopefully so many users are not having that experience 😬

adrian-codecov · 2024-08-01T16:09:56Z

@spalmurray-codecov it shouldn't be null yeaaaah, mmmm is your local api/worker/staging using the latest shared/built using the latest? If you see the error, it sources from an error being thrown because the customer doesn't have a valid token, so I guess we could interpret that as either false or null. Sounds like the resolver needs a try/except

codecov-staging · 2024-08-01T23:05:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

can return null

37db31d

ajay-sentry requested a review from a team as a code owner July 31, 2024 23:43

spalmurray-codecov approved these changes Aug 1, 2024

View reviewed changes

add UT and log

1554b39

ajay-sentry added 2 commits August 1, 2024 16:08

update this test

9ba524c

dont hardcode id

3c05e9c

ajay-sentry added this pull request to the merge queue Aug 2, 2024

Merged via the queue into main with commit 5a2c7f0 Aug 2, 2024
17 of 18 checks passed

ajay-sentry deleted the Ajay/fix-githubratelimited branch August 2, 2024 16:13

ajay-sentry mentioned this pull request Aug 5, 2024

Fix GraphQL isGithubRateLimited Sentry Issue codecov/engineering-team#2203

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: isGithubRateLimited can return null #728

fix: isGithubRateLimited can return null #728

ajay-sentry commented Jul 31, 2024

codecov-qa bot commented Jul 31, 2024 •

edited

Loading

pytest

codecov-public-qa bot commented Jul 31, 2024 •

edited

Loading

pytest

codecov bot commented Jul 31, 2024 •

edited

Loading

spalmurray-codecov commented Aug 1, 2024

adrian-codecov commented Aug 1, 2024 •

edited

Loading

codecov-staging bot commented Aug 1, 2024 •

edited by codecov-notifications bot

Loading

fix: isGithubRateLimited can return null #728

fix: isGithubRateLimited can return null #728

Conversation

ajay-sentry commented Jul 31, 2024

Purpose/Motivation

Notes to Reviewer

Legal Boilerplate

codecov-qa bot commented Jul 31, 2024 • edited Loading

❌ Failed Test Results:

pytest

codecov-public-qa bot commented Jul 31, 2024 • edited Loading

❌ Failed Test Results:

pytest

codecov bot commented Jul 31, 2024 • edited Loading

Codecov Report

spalmurray-codecov commented Aug 1, 2024

adrian-codecov commented Aug 1, 2024 • edited Loading

codecov-staging bot commented Aug 1, 2024 • edited by codecov-notifications bot Loading

Codecov Report

codecov-qa bot commented Jul 31, 2024 •

edited

Loading

codecov-public-qa bot commented Jul 31, 2024 •

edited

Loading

codecov bot commented Jul 31, 2024 •

edited

Loading

adrian-codecov commented Aug 1, 2024 •

edited

Loading

codecov-staging bot commented Aug 1, 2024 •

edited by codecov-notifications bot

Loading