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

Fixed issue with header function evaluation + added 2 tests #455

Merged
merged 1 commit into from
Jan 25, 2016

Conversation

alekbarszczewski
Copy link
Contributor

Fixes #454

  1. Reply header functions were evaluated twice per request.
  2. Reply header functions were overridden by evaluated values.

After fix each header function is evaluated only once per request.
After fix each header function is evaluated on each and every request separately.

@alekbarszczewski
Copy link
Contributor Author

Hmm, the problem with coverage is here: https://coveralls.io/builds/4824108/source?filename=lib%2Frequest_overrider.js#L461. I've coded it to be sure that both response.headers and response.rawHeaders will be evaluated. But it seems that interceptor.headers are identical to interceptor.rawHeaders, but in array (instead of object) representation. Is that true? If yes then I can change:

if (typeof value === "function") {
  // Check if header has not been already evaluated. Evaluate it otherwise.
  if (evaluatedHeaders.hasOwnProperty(key)) {
    response.rawHeaders[rawHeaderIndex + 1] = evaluatedHeaders[key]
  } else {
    response.rawHeaders[rawHeaderIndex + 1] = value(req, response, responseBody);
  }
}

to

if (typeof value === "function") {
  response.rawHeaders[rawHeaderIndex + 1] = evaluatedHeaders[key];
}

@pgte
Copy link
Member

pgte commented Jan 23, 2016

@alekbarszczewski agree, that should be enough.

@alekbarszczewski
Copy link
Contributor Author

@pgte Done, squashed additional commit, thanks.

@pgte pgte merged commit 24738e9 into nock:master Jan 25, 2016
@pgte
Copy link
Member

pgte commented Jan 25, 2016

@alekbarszczewski thanks!

Landed on v6.0.1.

@lock
Copy link

lock bot commented Sep 14, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants