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

fix(adapter): Log information if request couldn't be found in recording #172

Merged
merged 1 commit into from
Jan 31, 2019

Conversation

dustinsoftware
Copy link
Contributor

These three fields are referenced in the _identify function.

Fixes #114

@dustinsoftware
Copy link
Contributor Author

1 test appears to be a flake (adapter-fetch test timed out)

@offirgolan
Copy link
Collaborator

@dustinsoftware there are a couple of other places where we log

`${pollyRequest.method} ${pollyRequest.url}`

Can you replace all of those as well? A small utility method to stringify a request for logging would be good here 😄

@dustinsoftware
Copy link
Contributor Author

dustinsoftware commented Jan 29, 2019 via email

@dustinsoftware
Copy link
Contributor Author

Hmm. Just did a quick search, this is the only other use I can find. Won't this exception only be thrown if there is a bug in polly itself?

    // This should never be reached. If it did, then something screwy happened.
    this.assert(
      `Unhandled request: ${pollyRequest.method} ${pollyRequest.url}.`,
      false
    );

@offirgolan
Copy link
Collaborator

Oh... I totally thought there were a few other places but I was mistaken. My bad! Lets just add it there as well for consistency. If it does ever reach that point, at least the user will have more information about the request.

@dustinsoftware
Copy link
Contributor Author

Updated, let me know if there's any other things you'd like changed :)

These three fields are referenced in the _identify function.
The url field is included just in case it is not included already in
the identifiers.
@offirgolan offirgolan merged commit 8dcdf7b into Netflix:master Jan 31, 2019
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.

2 participants