adhearsion / voicemail

Adhearsion voicemail plugin
MIT License
8 stars 12 forks source link

hotfix/1.1.2 #39

Closed emcgee closed 10 years ago

emcgee commented 10 years ago

Quick hotfix to guarantee that the on_end callback is actually used. In the current configuration, Adhearsion can exit without ever seeing the call to setup on_end, thereby leaving voicemails to be delivered to nobody except the recording directory.

bklang commented 10 years ago

Great catch. Need to get the test that is now failing to pass, but otherwise this looks good.

emcgee commented 10 years ago

Spec fixed, but there is another slight problem:

I'm seeing quite a few undefined method "complete_event" errors on this line: https://github.com/adhearsion/voicemail/blob/develop/lib/voicemail/call_controllers/voicemail_controller.rb#L54

Likely because the call is shutting down before the the Record has time, or is instructed, to finish. Up for discussion what is best here - to sleep for a second or two and then process, or poll recording.complete_event for a certain amount of time and then hangup if nonexistent.

benlangfeld commented 10 years ago

@emcgee Please provide evidence of such errors. NoMethodError would not occur because the complete event is not yet set, but rather because @recording is nil. This would happen if hangup occurred while the greeting was being played back, for example. If that callback is going to be set before recording starts, it will need to account for the possibility that recording never starts.

emcgee commented 10 years ago

@benlangfeld Indeed, @recording is nil.

https://gist.github.com/emcgee/2ae85aca3afb68e3e637

Since the callback does need to be set before recording starts, I'll add another line to check for that condition.

emcgee commented 10 years ago

Any more issues with this, or can it be merged?

bklang commented 10 years ago

@emcgee it needs to be rebased to the support/1.x.x branch because develop has moved on to voicemail 2.0. I'm good with it otherwise.

emcgee commented 10 years ago

@bklang Squashed and rebased.