You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A new NodeDomain module, which is a high-level wrapper around NodeConnection. This makes it easy to open a connection and load a single domain, and then execute the domain's commands and listen for its events. The wrapper handles, in particular, connection and domain loading and reloading.
The inline documentation gives the following usage example:
var myDomain = new NodeDomain("someDomain", "/path/to/SomeDomainDef");
$result = myDomain.exec("someCommand", arg1, arg2);
$result.done(function (value) {
// the command succeeded!
});
$result.fail(function (err) {
// the command failed; act accordingly!
});
$(myDomain).on("someEvent", someHandler);
A refactoring of StaticServer to use NodeDomain instead of manipulating NodeConnection directly. The previous code had been copied and pasted regularly, but had a few shortcomings, including redundant timeouts (NodeConnection handles this already), failure to handle disconnected sockets, and, in some cases, a silent fail-fast behavior on command execution where simply waiting would suffice. (If the NodeConnection isn't connected, don't abort; just wait for it to come back up!)
Finally, it also fixes a race condition in StaticServerDomain that was causing the LiveDevelopment unit tests to fail unpredictably.
The problem is as follows: when StaticServerDomain receives an HTTP request for a filtered path, it emits a filterRequest event to Brackets to have the file contents at that path manipulated before sending the response. Brackets later calls the writeFilteredResponse command with the manipulated data, which triggers the HTTP response. StaticServerDomain connects these filtered responses to their original filter requests by way of a map of callbacks keyed on the requested paths. But, as a consequence, when the StaticServerDomain HTTP server receives two concurrent requests for the same path, it only stores one response callback. The second filtered response is then be dropped, no longer having any response callback at the given path, and the second request---whose response callback has been overwritten---will always time out. The problem is solved here by adding a unique identifier to each filterRequest and keying callbacks on this identifier.
The reason will be displayed to describe this comment to others. Learn more.
Is this right? Looking at the usage it should be {Object.<string, {Object.<number, function>}} and A map from root paths to its request to reponse callback mapping.
Interesting. I just happened to have an open live preview session in Brackets when I ran the StaticServer unit tests. I saw about half of them fail. However, the tests did pass after I closed the session.
The reason will be displayed to describe this comment to others. Learn more.
Maybe not? I wasn't quite sure, so I opted to try to preserve the existing functionality. Everything is so delicate with live development, so when in doubt I just tried not to shake anything. If you don't think it's necessary though I could also kill that promise() method...
The only reason I didn't convert more NodeConnection clients to NodeDomain was time. Also I didn't think you'd merge it unless I refactored at least one client :)
Ready for re-review. I removed the StaticServer dependency on NodeDomain.promise() but left that method in there, thinking that it might be useful to someone else. But it's currently unused code, so I'm perfectly happy to remove that too. Whatever!
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request has three parts:
NodeDomain
module, which is a high-level wrapper aroundNodeConnection
. This makes it easy to open a connection and load a single domain, and then execute the domain's commands and listen for its events. The wrapper handles, in particular, connection and domain loading and reloading.The inline documentation gives the following usage example:
StaticServer
to useNodeDomain
instead of manipulatingNodeConnection
directly. The previous code had been copied and pasted regularly, but had a few shortcomings, including redundant timeouts (NodeConnection
handles this already), failure to handle disconnected sockets, and, in some cases, a silent fail-fast behavior on command execution where simply waiting would suffice. (If theNodeConnection
isn't connected, don't abort; just wait for it to come back up!)StaticServerDomain
that was causing theLiveDevelopment
unit tests to fail unpredictably.The problem is as follows: when
StaticServerDomain
receives an HTTP request for a filtered path, it emits afilterRequest
event to Brackets to have the file contents at that path manipulated before sending the response. Brackets later calls thewriteFilteredResponse
command with the manipulated data, which triggers the HTTP response.StaticServerDomain
connects these filtered responses to their original filter requests by way of a map of callbacks keyed on the requested paths. But, as a consequence, when theStaticServerDomain
HTTP server receives two concurrent requests for the same path, it only stores one response callback. The second filtered response is then be dropped, no longer having any response callback at the given path, and the second request---whose response callback has been overwritten---will always time out. The problem is solved here by adding a unique identifier to eachfilterRequest
and keying callbacks on this identifier.