Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

Fix 16550 "not authorized", TypeError, and others #324

Merged
merged 5 commits into from
Oct 16, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/moped/authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def apply_credentials(logins)
login(database, username, password)
end
end
@original_credentials = credentials.dup
end
self
end
Expand Down
14 changes: 11 additions & 3 deletions lib/moped/cluster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@ def refresh(nodes_to_refresh = seeds)
seen = {}
# Set up a recursive lambda function for refreshing a node and it's peers.
refresh_node = ->(node) do
unless seen[node]
unless node.address.resolved
begin
node.refresh
rescue Errors::ConnectionFailure
end
end
unless seen[node] || !node.address.resolved
seen[node] = true
# Add the node to the global list of known nodes.
seeds.push(node) unless seeds.include?(node)
Expand Down Expand Up @@ -369,8 +375,10 @@ def initialize_copy(_)
# @since 1.0.0
def refresh_peers(node, &block)
node.peers.each do |node|
block.call(node) unless seeds.include?(node)
peers.push(node) unless peers.include?(node)
if node.address.resolved
block.call(node) unless seeds.include?(node)
peers.push(node) unless peers.include?(node)
end
end
end
end
Expand Down
7 changes: 5 additions & 2 deletions lib/moped/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def alive?
#
# @since 1.0.0
def connect
credentials.clear
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we clear credentials on connect now , instead of on disconnect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arthurnn If the socket disconnects (network issues, mongod failover, etc.), with_connection() can trigger a reconnect (sometimes without disconnect() being called). In this situation, credentials haven't been cleared and so they won't be resent, which causes the 16550 not authorized errors.

Resetting credentials ensures they're are always resent on reconnection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was brought up in my pull #286 which was rejected due to it causing some issues on 2.0. @zarqman hits the nail on the head on the problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sock = if !!options[:ssl]
Socket::SSL.connect(host, port, timeout)
else
Expand Down Expand Up @@ -74,7 +75,6 @@ def connected?
#
# @since 1.0.0
def disconnect
credentials.clear
@sock.close
rescue
ensure
Expand Down Expand Up @@ -217,7 +217,10 @@ def read_data(socket, length)
#
# @since 1.3.0
def with_connection
connect if @sock.nil? || [email protected]?
if @sock.nil? || [email protected]?
connect
apply_credentials(@original_credentials || {})
end
yield @sock
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/moped/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def down?
#
# @since 1.2.0
def disconnect
connection{ |conn| conn.disconnect }
connection{ |conn| conn.disconnect } if address.resolved
true
end

Expand Down Expand Up @@ -175,9 +175,9 @@ def ensure_connected(&block)

begin
connection do |conn|
stack(:connection) << conn
connect(conn) unless conn.connected?
connect(conn) unless conn.alive?
conn.apply_credentials(@credentials)
stack(:connection) << conn
yield(conn)
end
rescue Exception => e
Expand Down
1 change: 1 addition & 0 deletions spec/moped/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@
before do
node.connection do |conn|
conn.stub(:connected?).and_return(true)
conn.stub(:alive?).and_return(false)
conn.instance_variable_set(:@sock, nil)
end
end
Expand Down