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

Added logging for ResourceException error in NS1 requests. #71

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

jdickson0296
Copy link
Contributor

Added Exception to catch ResourceException error to get more details on which specific method failed.

2023-06-01T16:11:52  [139903552284480] INFO  Ns1Provider[ns1] apply: making 1 changes to linkedin.com.
Traceback (most recent call last):
  File "/home/vsts/work/1/s/dns/env/bin/octodns-sync", line 8, in <module>
    sys.exit(main())
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns/cmds/sync.py", line 57, in main
    manager.sync(
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns/manager.py", line 653, in sync
    total_changes += target.apply(plan)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns/provider/base.py", line 245, in apply
    self._apply(plan)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns_ns1/__init__.py", line 1727, in _apply
    getattr(self, f'_apply_{class_name}')(ns1_zone, change)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns_ns1/__init__.py", line 1683, in _apply_Update
    self._client.records_update(zone, domain, _type, **params)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns_ns1/__init__.py", line 113, in call
    new_record = func(self, zone, domain, _type, **params)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns_ns1/__init__.py", line 254, in records_update
    return self._try(self._records.update, zone, domain, _type, **params)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/octodns_ns1/__init__.py", line 269, in _try
    return method(*args, **kwargs)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/ns1/rest/records.py", line 146, in update
    return self._make_request(
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/ns1/rest/resource.py", line 75, in _make_request
    return self._transport.send(type, self._make_url(path), **kwargs)
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/ns1/rest/transport/requests.py", line 112, in send
    resp_headers, jsonOut = self._send(
  File "/home/vsts/work/1/s/dns/env/lib/python3.8/site-packages/ns1/rest/transport/requests.py", line 81, in _send
    raise ResourceException("server error", resp, resp.text)
ns1.rest.errors.ResourceException: server error
##[error]Bash exited with code '1'. 
  • Exception will log at the debug level the NS1 method that was attempted, the arguments passed, and the error message.
  • Added method names for the mocked calls used in related unit tests.
  • Effected unit tests: test_populate, test_sync

Copy link
Contributor

@ross ross left a comment

Choose a reason for hiding this comment

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

What situation were you running into this with?

Comment on lines 287 to 292
self.log.debug(
"_try: method=%s, args=%s, error=%s",
method.__name__,
str(args),
e.message,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable. Switching the debug to an exception and removing the e.message bit as I believe self.log.exception should take care of that and always print w/o having to add --debug which makes sense for an error.

Suggested change
self.log.debug(
"_try: method=%s, args=%s, error=%s",
method.__name__,
str(args),
e.message,
)
self.log.exception(
"_try: method=%s, args=%s, error=%s",
method.__name__,
str(args),
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense, I can add that change.

I would like to log the response and body to view information about the request.

            except ResourceException as e:
                self.log.exception(
                    "_try: method=%s, args=%s, response=%s, body=%s",
                    method.__name__,
                    str(args),
                    e.response,
                    e.body,
                )
                raise
2023-10-25T15:53:37  [13073186816] ERROR NS1Client _try: method=retrieve, args=('olinkedin.com',), response=<Response [401]>, body={"message":"Invalid NSONE Key","details":[{"type":"request-id","message":"0a9ed58c-310b-4b70-ae4d-ad7543c12d68"}]}

@jdickson0296
Copy link
Contributor Author

What situation were you running into this with?

It was related to the NS1 API being down and the engineer on our team couldn't gather any details about the request made to share with NS1 support.

@jdickson0296 jdickson0296 marked this pull request as ready for review October 25, 2023 23:12
@ross ross merged commit f0ec25e into octodns:main Oct 26, 2023
7 checks passed
@jdickson0296 jdickson0296 deleted the log-provider-requests branch October 27, 2023 16:42
@istr
Copy link
Contributor

istr commented Oct 27, 2023

@jdickson0296 very helpful. If I remember correctly, there was an announced maintenance window recently. And the next one is coming soon. 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants