fix: proxy events into instance var and add test script #1661
Conversation
|
@DonutEspresso, @hekike : I created a new branch. Instead of rebase my previous branch. I accidentally clicked over update branch on the github uI. Which will add non-essential commits. I will take care from next time. |
|
Thanks @akoserwal! Apologies for the trouble. I left some minor comments I copied over from the last PR- let me know if they make sense! |
| var server = restify.createServer({ | ||
| handleUpgrades: false | ||
| }); | ||
| t.equal(server.proxyEvents.length, 7); |
There was a problem hiding this comment.
I think I expect handleUpgrades: true to be 7, and 6 for false (may be related to the && indexOf check above)
There was a problem hiding this comment.
As per the doc
options.handleUpgrades Boolean Hook the upgrade event from the node HTTP server, pushing Connection: Upgrade requests through the regular request handling chain. (optional, default false) (http://restify.com/docs/server-api/).
We are updating proxyEvents array when
options.handleUpgrades:false
size will be 7 and in-case of true. we will skip it.
There was a problem hiding this comment.
Ah, I can't read. Must have been a long day. :D
lib/server.js
Outdated
| @@ -187,10 +187,10 @@ function Server(options) { | |||
|
|
|||
| this.router.on('mount', this.emit.bind(this, 'mount')); | |||
|
|
|||
| if (!options.handleUpgrades && PROXY_EVENTS.indexOf('upgrade') === -1) { | |||
| PROXY_EVENTS.push('upgrade'); | |||
| if (!options.handleUpgrades && this.proxyEvents.indexOf('upgrade') === -1) { | |||
There was a problem hiding this comment.
Now that proxyEvents is specific to the instance of the server being created, I don't think we even need the indexOf check right?
There was a problem hiding this comment.
@DonutEspresso: right, I can remove it.
| var server = restify.createServer({ | ||
| handleUpgrades: false | ||
| }); | ||
| t.equal(server.proxyEvents.length, 7); |
There was a problem hiding this comment.
Ah, I can't read. Must have been a long day. :D
|
@akoserwal thank you for the contribution! 🎉 |
|
@DonutEspresso : Thank you :) |
Pre-Submission Checklist
make prepushIssues
#1348
Closes:
Changes