Skip to content
GitLab
    • Explore Projects Groups Snippets
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • N node-http-proxy
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 482
    • Issues 482
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 102
    • Merge requests 102
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • http ... PARTY!
  • node-http-proxy
  • Merge requests
  • !503
An error occurred while fetching the assigned milestone of the selected merge_request.

Fixed issue where error callback would not invoke, including new test cases.

  • Review changes

  • Download
  • Email patches
  • Plain diff
Administrator requested to merge github/fork/mmoulton/caronte into caronte 11 years ago
  • Overview 8
  • Commits 3
  • Pipelines 0
  • Changes 2

Created by: mmoulton

The web/ws handlers allow a callback to be passed in that will be invoked on error. This callback was never being fired. This change fixes this by invoking the callback if present, otherwise emitting the 'error' event.

Included some new test cases to evaluate this state where the web/ws handlers are being used by an existing server. (ie: listen is never called on the proxy)

Loading
Loading

Activity


  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: coveralls

    Coverage Status

    Coverage increased (+2.0%) when pulling 37e390d4 on mmoulton:caronte into f23bbbaf on nodejitsu:caronte.

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: yawnt

    lookin good! would you mind moving the tests from the file you created to https://github.com/nodejitsu/node-http-proxy/blob/caronte/test/lib-http-proxy-passes-web-incoming-test.js ? we're trying to keep a 1-1 test-file js-file ratio :)

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: cronopio

    I was working on fixing this some hours ago, thank you so much. I did a test case with an error event handler but I miss the callback test, thanks for add it, follow the patter that @yawnt suggest and we'll merge this asap

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: mmoulton

    @yawnt @cronopio I have moved the tests into a single file. Let me know if there is anything else you would like to see. Thanks!

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: coveralls

    Coverage Status

    Coverage remained the same when pulling fc1a6dd6 on mmoulton:caronte into 5d66ce11 on nodejitsu:caronte.

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: mmoulton

    Got ya. I was assuming that since I was testing from the createProxyServer method down and not just the changes I made to web-incoming that you would want them in that file. I can move them if you would like.

    On Oct 21, 2013, at 2:48 PM, yawnt notifications@github.com wrote:

    hey @mmoulton, thanks for doing this, although you got the wrong file xD.. i need them to be in https://github.com/nodejitsu/node-http-proxy/blob/caronte/test/lib-http-proxy-passes-web-incoming-test.js since it's where you modified the code :)

    — Reply to this email directly or view it on GitHub.

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: yawnt

    it's ok, we'll cherry-picked.. thanks!

  • Administrator
    Administrator @root · 11 years ago
    Author Contributor

    Created by: cronopio

    Cherry-picked. Tests pass, Thanks!

  • Administrator closed 11 years ago

    closed

Please register or sign in to reply
0 Assignees
None
Assign to
0 Reviewers
None
Request review from
Labels
0
None
0
None
    Assign labels
  • Manage project labels

Milestone
No milestone
None
None
Time tracking
No estimate or time spent
Lock merge request
Unlocked
2
2 participants
Administrator
Manuel Astudillo
Reference:
Source branch: github/fork/mmoulton/caronte

Menu

Explore Projects Groups Snippets