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

Weekly Digest (2 June, 2019 - 9 June, 2019) #1582

Closed
weekly-digest bot opened this issue Jun 9, 2019 · 0 comments
Closed

Weekly Digest (2 June, 2019 - 9 June, 2019) #1582

weekly-digest bot opened this issue Jun 9, 2019 · 0 comments

Comments

@weekly-digest
Copy link

weekly-digest bot commented Jun 9, 2019

Here's the Weekly Digest for nock/nock:


ISSUES

Last week 5 issues were created.
Of these, 4 issues have been closed and 1 issues are still open.

OPEN ISSUES

💚 #1581 Update prettier to the latest version 🚀, by greenkeeper[bot]

CLOSED ISSUES

❤️ #1580 docs: Add note about OpenCollective bot, by RichardLitt
❤️ #1579 refactor: special asterisk bodies in definitions., by mastermatt
❤️ #1578 docs: Remove unnecessary uses of filteringRequestBody., by mastermatt
❤️ #1577 Add tests to increase coverage of the Request Overrider., by mastermatt

LIKED ISSUE

👍 #1578 docs: Remove unnecessary uses of filteringRequestBody., by mastermatt
It received 👍 x1, 😄 x0, 🎉 x0 and ❤️ x1.

NOISY ISSUE

🔈 #1577 Add tests to increase coverage of the Request Overrider., by mastermatt
It received 3 comments.


PULL REQUESTS

Last week, 9 pull requests were created, updated or merged.

UPDATED PULL REQUEST

Last week, 2 pull requests were updated.
💛 #1581 Update prettier to the latest version 🚀, by greenkeeper[bot]
💛 #1235 [NockBack] Record "unmatched" requests only., by guerrerocarlos

MERGED PULL REQUEST

Last week, 7 pull requests were merged.
💜 #1580 docs: Add note about OpenCollective bot, by RichardLitt
💜 #1579 refactor: special asterisk bodies in definitions., by mastermatt
💜 #1578 docs: Remove unnecessary uses of filteringRequestBody., by mastermatt
💜 #1577 Add tests to increase coverage of the Request Overrider., by mastermatt
💜 #1574 Throw if headersFieldNamesToLowerCase is not provided an object., by mastermatt
💜 #1570 Lint: Remove overrides., by mastermatt
💜 #1564 Response headers/raw headers to more closely match Node's functionality., by mastermatt


COMMITS

Last week there were 7 commits.
🛠️ [feat: Throw error if request headers are not an object (#1574) Throw if headersFieldNamesToLowerCase is not provided an object.
Adds a test to cover the exception case.

Going through the git history for this function, all the way back to
when it was added with 4048734, I can't find a case where a non-object
input was valid.

Once PR # 1564 is merged in, this utility will only be called in two
places:

  • Creating an Interceptor if the Scope was created with options
  • When http.request is called with an options object

In both places, providing defined, but non-object values causes chaos.
If the value is a truthy, non-iterable then the header was ignored
during matching. For example, the following will match Interceptors as
if reqheaders had not been provided.

nock('http://example.com', { reqheaders: 1 }).get('/').reply

However, if the value was provided as a non-plain object iterable, such
as an array or string, the header matching logic would attempt to match
header field names with numerical keys, rendering the whole Scope
useless.](8ba0fc7) by mastermatt
🛠️ [refactor: special asterisk bodies in definitions. (#1579) - Clarify and add test of definitions bodies that are simply "*".

Updates the header handling in the Interceptor and RequestOverrider with
the intention of mimicking the native behavior of http.IncomingMessage.rawHeaders.

The raw request/response headers list exactly as they were received.

There are three fundamental changes in this changeset:

  1. Header Input Type

Previously, headers could be provided to:

  • Scope.defaultReplyHeaders as a plain object
  • Interceptor.reply(status, body, headers) as a plain object or an array of raw headers
  • Interceptor.reply(() => [status, body, headers] as a plain object

Now, all three allow consistent inputs where the headers can be provided as a
plain object, an array of raw headers, or a Map.

  1. Duplicate Headers Folding

This change deviates from the suggested guidelines laid out in #1553 because
those guidelines didn't properly handle duplicate headers, especially when
some are defined as defaults.

This change was modeled to duplicate Node's implementation
(relevant docs).
It specifically lays out how duplicate headers are handled depending on the field name.

In the case of default headers, they are not included on the Response
(not even in the raw headers) if the field name exists in the reply headers
(using a case-insensitive comparison).

  1. Raw Headers are the Source of Truth

Previously, the Interceptor and RequestOverrider mostly keep track of
headers as a plain object and the array of raw headers was created by looping
that object. This was the cause for inconsistencies with the final result of
the raw headers. The problem with that approach is that converting raw headers
to an object is a lossy process, so going backwards makes it impossible to
guarantee the correct results.

This change reverses that logic and now the Interceptor and RequestOverrider
maintain the header data in raw arrays. All additions to headers are only added
to raw headers. The plain headers object is never mutated directly, and instead
is [re]created from the raw headers as needed.](b687592) by mastermatt
🛠️ docs: Add note about OpenCollective bot (#1580) by RichardLitt
🛠️ [docs: Remove unnecessary uses of filteringRequestBody. (#1578) Before v2.0, it was not possible to not filter on request bodies.

For users wanting to match regardless of the request body, a convention
became the norm of using filteringRequestBody to modify the body into
a special string (*) then match against that specific string in the
Interceptor.

In more recent versions, the preferred way to not match request bodies
is to not provide a body argument when creating an Interceptor. This
updates the docs to reflect that.](8e672aa) by mastermatt
🛠️ [style: Remove most of the eslint rule overrides. (#1570) * chore(lint): Remove override: no-path-concat.

There was no code needed to change. 🤷

  • chore(lint): Remove override: no-throw-literal.

  • chore(lint): Remove override: eqeqeq.

Require use of type-safe equality operators.

  • chore(lint): Remove override: new-cap.

http.request doesn't use a new keyword anyway.
https://nodejs.org/api/http.html#http_http_request_options_callback

  • chore(lint): Remove override: camelcase.

  • chore(lint): Remove override: handle-callback-err.

  • chore(lint): Remove unnecessary eslint-disable strict.

  • Apply suggestions from code review

Co-Authored-By: Paul Melnikow github@paulmelnikow.com

  • Code review updates.

  • Update lib/common.js

Co-Authored-By: Paul Melnikow github@paulmelnikow.com](7e5403b) by mastermatt
🛠️ [refactor(request_overrider): Remove unreachable code and increase coverage * Add test to cover multiple calls to continueWithResponseBody.

This check was originally added when fixing a bug related to aborted
requests after calling delayConnection.
28f6b29 "abort behaves. somehow addresses #439"

  • Remove unreachable block in respond.

Returning early from this function was moved to the top with 28f6b29,
the original check was left behind.

  • Add test to cover direct use of ClientRequest with an optional callback.

Because of the way we override http.request, this callback is only
defined when http.ClientRequest is called directly with a callback.

  • Add test to cover reply body streams proxy error events.

  • Add test to cover an error in a full reply callback.

  • Remove unnecessary try/catch in overrider.

In the normal error case, that the value of response body is a string, but
not hex encoded, the attempt to create a Buffer will return a Uint8Array
with nothing in it.

Looking at the Node source for Buffer.from, the reason errors are
thrown are:

  • The first arg isn't a valid type, but we've already ensured it's a string.
  • The encoding arg is not valid, but we hardcoded 'hex'.

CONTRIBUTORS

Last week there were 2 contributors.
👤 mastermatt
👤 RichardLitt


STARGAZERS

Last week there were 25 stagazers.
nhnam
piresten
micrub
anagorsky
lukasduspiva
YangSongPro
cyfootloose
welloni
meabed
smatjes
matthewmorek
flyingluscas
Edd1525
willsoto
pungggi
itsfadnis
tlwish
mikeletscher
corentinmarc
Calinou
cuchi
DittrichLucas
mantcz
niztal
don-siu
You all are the stars! 🌟


RELEASES

Last week there were 2 releases.
🚀 v11.0.0-beta.19@beta v11.0.0-beta.19@beta
🚀 v11.0.0-beta.18@beta v11.0.0-beta.18@beta


That's all for last week, please 👀 Watch and Star the repository nock/nock to receive next weekly updates. 😃

You can also view all Weekly Digests by clicking here.

Your Weekly Digest bot. 📆

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

No branches or pull requests

1 participant