From 2f335363b9a4b5e41d09ca2030181ae3521b3a1c Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 10 Aug 2021 13:44:21 -0500 Subject: [PATCH 1/2] When user cancels the command, respond with canceled If the cancel has not been fully handled in the body of the command, at least respond with canceled status and do not go to sentry. --- lib/command.rb | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/command.rb b/lib/command.rb index b4ce5b9dce1c8bc2015bbff79436c0ad0ff6fba4..e07616691cd8f3adeec8298070a91e5de2364516 100644 --- a/lib/command.rb +++ b/lib/command.rb @@ -103,23 +103,26 @@ class Command protected def catch_after(promise) - promise.catch_only(FinalStanza) { |e| + promise.catch_only(Blather::Stanza::Iq::Command) { |iq| + next EMPromise.reject(iq) unless iq.cancel? + + finish(status: :canceled) + }.catch_only(FinalStanza) { |e| @blather << e.stanza }.catch do |e| log_error(e) - finish( - @format_error.call(e), type: :error - ).catch_only(FinalStanza) do |to_send| - @blather << to_send.stanza - end + send_final_error(e) + end + end + + def send_final_error(e) + finish(@format_error.call(e), type: :error).catch_only(FinalStanza) do |s| + @blather << s.stanza end end def log_error(e) - @log.error( - "Error raised during #{iq.node}: #{e.class}", - e - ) + @log.error("Error raised during #{iq.node}: #{e.class}", e) if e.is_a?(::Exception) sentry_hub.capture_exception(e) else From 575da6dec6d43368af4c7b91765150ddc08ff854 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 10 Aug 2021 13:49:54 -0500 Subject: [PATCH 2/2] Timeout is not a fatal error If the user does not proceed with a command after N time, we don't hold on to it forever and time out. We cannot return anything to the user because they haven't sent us anything, so just ignore it. --- lib/command.rb | 9 +++++++-- lib/session_manager.rb | 39 +++++++++++++++++++++++++++++++++++++++ sgx_jmp.rb | 37 +------------------------------------ 3 files changed, 47 insertions(+), 38 deletions(-) create mode 100644 lib/session_manager.rb diff --git a/lib/command.rb b/lib/command.rb index e07616691cd8f3adeec8298070a91e5de2364516..97f6e1228a280ef70fc07ebff7c3fee7f0f6899f 100644 --- a/lib/command.rb +++ b/lib/command.rb @@ -4,6 +4,7 @@ require "sentry-ruby" require "statsd-instrument" require_relative "customer_repo" +require_relative "session_manager" class Command def self.execution @@ -27,6 +28,8 @@ class Command end class Execution + class Timeout < SessionManager::Timeout; end + class FinalStanza attr_reader :stanza @@ -59,8 +62,10 @@ class Command reply.status = :executing end yield stanza if block_given? - COMMAND_MANAGER.write(stanza).then do |new_iq| + COMMAND_MANAGER.write(stanza).then { |new_iq| @iq = new_iq + }.catch_only(SessionManager::Timeout) do + EMPromise.reject(Timeout.new) end end @@ -107,7 +112,7 @@ class Command next EMPromise.reject(iq) unless iq.cancel? finish(status: :canceled) - }.catch_only(FinalStanza) { |e| + }.catch_only(Timeout) {}.catch_only(FinalStanza) { |e| @blather << e.stanza }.catch do |e| log_error(e) diff --git a/lib/session_manager.rb b/lib/session_manager.rb new file mode 100644 index 0000000000000000000000000000000000000000..80b1ffe9dcbb66c4b7d358dd8301b224aef6deee --- /dev/null +++ b/lib/session_manager.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class SessionManager + class Timeout < StandardError; end + + def initialize(blather, id_msg, timeout: 5, error_if: nil) + @blather = blather + @sessions = {} + @id_msg = id_msg + @timeout = timeout + @error_if = error_if + end + + def promise_for(stanza) + id = "#{stanza.to.stripped}/#{stanza.public_send(@id_msg)}" + @sessions.fetch(id) do + @sessions[id] = EMPromise.new + EM.add_timer(@timeout) do + @sessions.delete(id)&.reject(Timeout.new) + end + @sessions[id] + end + end + + def write(stanza) + promise = promise_for(stanza) + @blather << stanza + promise + end + + def fulfill(stanza) + id = "#{stanza.from.stripped}/#{stanza.public_send(@id_msg)}" + if stanza.error? || @error_if&.call(stanza) + @sessions.delete(id)&.reject(stanza) + else + @sessions.delete(id)&.fulfill(stanza) + end + end +end diff --git a/sgx_jmp.rb b/sgx_jmp.rb index e44ecb83360a57619f1e0c8424a323df2a1ca995..2db02a23747847c4a45dc749af72bea933a2102a 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -72,6 +72,7 @@ require_relative "lib/payment_methods" require_relative "lib/registration" require_relative "lib/transaction" require_relative "lib/web_register_manager" +require_relative "lib/session_manager" require_relative "lib/statsd" ELECTRUM = Electrum.new(**CONFIG[:electrum]) @@ -275,42 +276,6 @@ message :error? do |m| LOG.error "MESSAGE ERROR", stanza: m end -class SessionManager - def initialize(blather, id_msg, timeout: 5, error_if: nil) - @blather = blather - @sessions = {} - @id_msg = id_msg - @timeout = timeout - @error_if = error_if - end - - def promise_for(stanza) - id = "#{stanza.to.stripped}/#{stanza.public_send(@id_msg)}" - @sessions.fetch(id) do - @sessions[id] = EMPromise.new - EM.add_timer(@timeout) do - @sessions.delete(id)&.reject(:timeout) - end - @sessions[id] - end - end - - def write(stanza) - promise = promise_for(stanza) - @blather << stanza - promise - end - - def fulfill(stanza) - id = "#{stanza.from.stripped}/#{stanza.public_send(@id_msg)}" - if stanza.error? || @error_if&.call(stanza) - @sessions.delete(id)&.reject(stanza) - else - @sessions.delete(id)&.fulfill(stanza) - end - end -end - IQ_MANAGER = SessionManager.new(self, :id) COMMAND_MANAGER = SessionManager.new( self,