fix(AutoCollectHttpRequests): Http requests aren't collect if we call dispose+setup after http.createServer #947
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.
- What is the problem?
Currently, every time an instance of AutoCollectHttpRequests is created, we overwrite the http(s).createServer function to add a handler to track HTTP(S) requests. As a result, if we call the constructor several times we end up with several handlers that can duplicate data.
The second problem is when we initialize the library once with setup() and then create the server with http(s).createServer. Here we track the requests correctly but if, for X reasons, we call dispose() and then setup(), the HTTP requests will not be tracked anymore because the code waits for a call to createServer to add the instance handler (despite the fact that the instance created at the first setup call already had one in place)
- How is it corrected?
As the code allows to have only one instance of AutoCollectHttpRequests at the same time ( stored in the static variable INSTANCE ), it is not necessary to modify the code of http(s).createServer each time the constructor is called. It is necessary to do it only once and in the added handler, we call a method of the instance stored in the static variable to register the request.
Thus, after the call to dispose() the registrations will stop but will resume after the call to setup despite the change of instance.
The fix is not optimized. In fact, this kind of handler should not be added when calling a constructor but rather at the very beginning of the library to optimize the chances that the library is well instantiated by the developers. However, after a discussion with @hectorhdzg ( #945 ) it turns out that this part of the code is going to change soon, so I didn't want to dwell on a code that will soon be completely overwritten.