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

feat: improve error output by showing which URL caused the error #1735

Merged
merged 4 commits into from Oct 7, 2019

Conversation

martinfrancois
Copy link
Contributor

After getting an error in the interceptor, mentioning URL's need to start with a slash, it was difficult to find out which URL was causing the issue. With this change, since the URL is included in the error message, it's easier to troubleshoot the issue.

Copy link
Member

@paulmelnikow paulmelnikow left a comment

Choose a reason for hiding this comment

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

Thanks for this!

lib/interceptor.js Outdated Show resolved Hide resolved
@paulmelnikow
Copy link
Member

I don't have access to push the suggested change, though if you accept it I can merge.

Co-Authored-By: Paul Melnikow <github@paulmelnikow.com>
@paulmelnikow paulmelnikow merged commit c0be8f8 into nock:beta Oct 7, 2019
@nockbot
Copy link
Collaborator

nockbot commented Oct 7, 2019

🎉 This PR is included in version 11.1.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@paulmelnikow
Copy link
Member

Oh gosh, this was targeting beta. What have I done?! 🤦‍♂

@paulmelnikow
Copy link
Member

@martinfrancois Would you mind opening a new PR targeting master?

@paulmelnikow
Copy link
Member

/cc @gr2m

@gr2m
Copy link
Member

gr2m commented Oct 7, 2019

I’d try to cherry pick the commits into a branch based of master and the send a PR

@martinfrancois
Copy link
Contributor Author

martinfrancois commented Oct 7, 2019

@paulmelnikow Sorry, didn't know this was supposed to go against master! Just reopened a PR with the change cherry picked against master (like suggested by @gr2m), see #1740

martinfrancois added a commit to martinfrancois/nock that referenced this pull request Oct 7, 2019
…k#1735)

After getting an error in the interceptor, mentioning URL's need to start with a slash, it was difficult to find out which URL was causing the issue. With this change, since the URL is included in the error message, it's easier to troubleshoot the issue.
martinfrancois added a commit to martinfrancois/nock that referenced this pull request Oct 7, 2019
…k#1735)

After getting an error in the interceptor, mentioning URL's need to start with a slash, it was difficult to find out which URL was causing the issue. With this change, since the URL is included in the error message, it's easier to troubleshoot the issue.
martinfrancois added a commit to martinfrancois/nock that referenced this pull request Oct 7, 2019
…k#1735)

After getting an error in the interceptor, mentioning URL's need to start with a slash, it was difficult to find out which URL was causing the issue. With this change, since the URL is included in the error message, it's easier to troubleshoot the issue.
paulmelnikow pushed a commit that referenced this pull request Oct 7, 2019
…) (#1740)

After getting an error in the interceptor, mentioning URL's need to start with a slash, it was difficult to find out which URL was causing the issue. With this change, since the URL is included in the error message, it's easier to troubleshoot the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants