Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook should not replace request #291

Closed
kalarani opened this issue May 29, 2012 · 4 comments
Closed

Hook should not replace request #291

kalarani opened this issue May 29, 2012 · 4 comments
Milestone

Comments

@kalarani
Copy link

Right now the Savon::Hooks::Hook replaces the POST request executed to call a service. Ideally I would expect a hook to be called before/after/around the service call, not to replace the service call itself.

I can return false from my hook and let the Savon::SOAP::Request class make the request by itself. But that'll be useful only if I'm interested in doing some stuff before the call.

Hooks should be improvised to let user hook into events like 'before_request', 'after_request' etc.,

@kalarani
Copy link
Author

kalarani commented Jun 4, 2012

Have explained the issue with hooks here, where I'm trying to measure time taken by each soap request.

Since Savon::Hooks::Hook replaces the request, I have to place the request from the hook itself, though I'm just interested in hooks just before the call is placed and after the response is received.

@rubiii rubiii closed this as completed in 9817cfd Jun 8, 2012
rubiii referenced this issue in savonrb/savon_spec Jun 8, 2012
introduced by rubiii/savon#291
@rubiii
Copy link
Contributor

rubiii commented Jun 8, 2012

could you test whether this change works for you? there's a spec describing your use case.
basically you should be able to replace your code with:

Savon.configure do |c|
  c.hooks.define('new_hook', :soap_request) do |callback, req|
    start_time = Time.now
    response = callback.call
    SavonInstrument.add req.http.headers['SoapAction'], Time.now - start_time
    response
  end
end

@kalarani
Copy link
Author

kalarani commented Jun 8, 2012

It works!! Thank you!!

@rubiii
Copy link
Contributor

rubiii commented Jun 8, 2012

you're welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants