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.
Hello :)
Events implementation as proposed in 214 seems to work incorrectly.
Issue 1:
Because a
callback
function is passed to plenary's curl, the request will be made asynchronously, causing theRestStopRequest
event to be emitted right after thelocal success_req, req_err = pcall(curl.curl_cmd, Opts)
returns, way before the actual request finishes.Issue 2:
The events are emitted only during normal
run_request
, they are not emitted during rerunning thelast
request.Proposed changes:
curl/init.lua
file. This way it won't matter if the call is made viarun_request
orlast
method. I'm actually not sure about his one though. Maybe it would be better to extract the events back to the maininit.lua
file, however it was tempting how easy it was to implement.RestStopRequest
inside the curl callback function.on_error
handler for plenary's curl. This allows us to emit theRestStopRequest
event, when the request fails (.e.g. could not resolve the URL)The proposed changes will alter:
RestStartRequest
andRestStopRequest
- this can be some what mitigated I guess. However might be not a big deal, if the feature didn't worked correctly in the first place.error(err.message)
increate_error_handler
Ofc. I'm open to any suggestions :)
I didn't had the time to check if this is in line with other PR that are currently open :(