Skip to content

Commit 9a0b56a

Browse files
committed
CSHARP-2403: Log any unexpected exceptions that occur in SDAM.
1 parent 81da63e commit 9a0b56a

File tree

3 files changed

+125
-13
lines changed

3 files changed

+125
-13
lines changed

src/MongoDB.Driver.Core/Core/Clusters/MultiServerCluster.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,31 @@ private async Task MonitorServersAsync()
227227
var eventArgs = await _serverDescriptionChangedQueue.DequeueAsync(monitorServersCancellationToken).ConfigureAwait(false); // TODO: add timeout and cancellationToken to DequeueAsync
228228
ProcessServerDescriptionChanged(eventArgs);
229229
}
230-
catch
230+
catch (OperationCanceledException) when (monitorServersCancellationToken.IsCancellationRequested)
231231
{
232-
// TODO: log this somewhere...
232+
// ignore OperationCanceledException when monitor servers cancellation is requested
233+
}
234+
catch (Exception unexpectedException)
235+
{
236+
// if we catch an exception here it's because of a bug in the driver
237+
238+
var handler = _sdamInformationEventHandler;
239+
if (handler != null)
240+
{
241+
try
242+
{
243+
handler.Invoke(new SdamInformationEvent(() =>
244+
string.Format(
245+
"Unexpected exception in MultiServerCluster.MonitorServersAsync: {0}",
246+
unexpectedException.ToString())));
247+
}
248+
catch
249+
{
250+
// ignore any exceptions thrown by the handler (note: event handlers aren't supposed to throw exceptions)
251+
}
252+
}
253+
254+
// TODO: should we reset the cluster state in some way? (the state is undefined since an unexpected exception was thrown)
233255
}
234256
}
235257
}

src/MongoDB.Driver.Core/Core/Servers/ServerDescription.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,5 +559,37 @@ public ServerDescription With(
559559
return this;
560560
}
561561
}
562+
563+
/// <summary>
564+
/// Returns a new ServerDescription with a new HeartbeatException.
565+
/// </summary>
566+
/// <param name="heartbeatException">The heartbeat exception.</param>
567+
/// <returns>
568+
/// A new instance of ServerDescription.
569+
/// </returns>
570+
public ServerDescription WithHeartbeatException(Exception heartbeatException)
571+
{
572+
return new ServerDescription(
573+
_serverId,
574+
_endPoint,
575+
averageRoundTripTime: _averageRoundTripTime,
576+
canonicalEndPoint: _canonicalEndPoint,
577+
electionId: _electionId,
578+
heartbeatException: heartbeatException,
579+
heartbeatInterval: _heartbeatInterval,
580+
lastUpdateTimestamp: _lastUpdateTimestamp,
581+
lastWriteTimestamp: _lastWriteTimestamp,
582+
logicalSessionTimeout: _logicalSessionTimeout,
583+
maxBatchCount: _maxBatchCount,
584+
maxDocumentSize: _maxDocumentSize,
585+
maxMessageSize: _maxMessageSize,
586+
maxWireDocumentSize: _maxWireDocumentSize,
587+
replicaSetConfig: _replicaSetConfig,
588+
state: _state,
589+
tags: _tags,
590+
type: _type,
591+
version: _version,
592+
wireVersionRange: _wireVersionRange);
593+
}
562594
}
563595
}

src/MongoDB.Driver.Core/Core/Servers/ServerMonitor.cs

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ internal sealed class ServerMonitor : IServerMonitor
4747
private readonly Action<ServerHeartbeatStartedEvent> _heartbeatStartedEventHandler;
4848
private readonly Action<ServerHeartbeatSucceededEvent> _heartbeatSucceededEventHandler;
4949
private readonly Action<ServerHeartbeatFailedEvent> _heartbeatFailedEventHandler;
50+
private readonly Action<SdamInformationEvent> _sdamInformationEventHandler;
5051

5152
public event EventHandler<ServerDescriptionChangedEventArgs> DescriptionChanged;
5253

@@ -64,6 +65,7 @@ public ServerMonitor(ServerId serverId, EndPoint endPoint, IConnectionFactory co
6465
eventSubscriber.TryGetEventHandler(out _heartbeatStartedEventHandler);
6566
eventSubscriber.TryGetEventHandler(out _heartbeatSucceededEventHandler);
6667
eventSubscriber.TryGetEventHandler(out _heartbeatFailedEventHandler);
68+
eventSubscriber.TryGetEventHandler(out _sdamInformationEventHandler);
6769
}
6870

6971
public ServerDescription Description => Interlocked.CompareExchange(ref _currentDescription, null, null);
@@ -91,7 +93,7 @@ public void Initialize()
9193

9294
public void Invalidate()
9395
{
94-
OnDescriptionChanged(_baseDescription);
96+
SetDescriptionIfChanged(_baseDescription);
9597
RequestHeartbeat();
9698
}
9799

@@ -113,7 +115,47 @@ private async Task MonitorServerAsync()
113115
{
114116
try
115117
{
116-
await HeartbeatAsync(heartbeatCancellationToken).ConfigureAwait(false);
118+
try
119+
{
120+
await HeartbeatAsync(heartbeatCancellationToken).ConfigureAwait(false);
121+
}
122+
catch (OperationCanceledException) when (heartbeatCancellationToken.IsCancellationRequested)
123+
{
124+
// ignore OperationCanceledException when heartbeat cancellation is requested
125+
}
126+
catch (Exception unexpectedException)
127+
{
128+
// if we catch an exception here it's because of a bug in the driver (but we need to defend ourselves against that)
129+
130+
var handler = _sdamInformationEventHandler;
131+
if (handler != null)
132+
{
133+
try
134+
{
135+
handler.Invoke(new SdamInformationEvent(() =>
136+
string.Format(
137+
"Unexpected exception in ServerMonitor.MonitorServerAsync: {0}",
138+
unexpectedException.ToString())));
139+
}
140+
catch
141+
{
142+
// ignore any exceptions thrown by the handler (note: event handlers aren't supposed to throw exceptions)
143+
}
144+
}
145+
146+
// since an unexpected exception was thrown set the server description to Unknown (with the unexpected exception)
147+
try
148+
{
149+
// keep this code as simple as possible to keep the surface area with any remaining possible bugs as small as possible
150+
var newDescription = _baseDescription.WithHeartbeatException(unexpectedException); // not With in case the bug is in With
151+
SetDescription(newDescription); // not SetDescriptionIfChanged in case the bug is in SetDescriptionIfChanged
152+
}
153+
catch
154+
{
155+
// if even the simple code in the try throws just give up (at least we've raised the unexpected exception via an SdamInformationEvent)
156+
}
157+
}
158+
117159
var newHeartbeatDelay = new HeartbeatDelay(metronome.GetNextTickDelay(), __minHeartbeatInterval);
118160
var oldHeartbeatDelay = Interlocked.Exchange(ref _heartbeatDelay, newHeartbeatDelay);
119161
if (oldHeartbeatDelay != null)
@@ -198,7 +240,7 @@ private async Task<bool> HeartbeatAsync(CancellationToken cancellationToken)
198240
newDescription = newDescription.With(heartbeatException: heartbeatException);
199241
}
200242

201-
OnDescriptionChanged(newDescription);
243+
SetDescriptionIfChanged(newDescription);
202244

203245
return true;
204246
}
@@ -257,15 +299,8 @@ private async Task<HeartbeatInfo> GetHeartbeatInfoAsync(IConnection connection,
257299
}
258300
}
259301

260-
private void OnDescriptionChanged(ServerDescription newDescription)
302+
private void OnDescriptionChanged(ServerDescription oldDescription, ServerDescription newDescription)
261303
{
262-
var oldDescription = Interlocked.CompareExchange(ref _currentDescription, null, null);
263-
if (oldDescription.Equals(newDescription))
264-
{
265-
return;
266-
}
267-
Interlocked.Exchange(ref _currentDescription, newDescription);
268-
269304
var handler = DescriptionChanged;
270305
if (handler != null)
271306
{
@@ -275,6 +310,29 @@ private void OnDescriptionChanged(ServerDescription newDescription)
275310
}
276311
}
277312

313+
private void SetDescription(ServerDescription newDescription)
314+
{
315+
var oldDescription = Interlocked.CompareExchange(ref _currentDescription, null, null);
316+
SetDescription(oldDescription, newDescription);
317+
}
318+
319+
private void SetDescription(ServerDescription oldDescription, ServerDescription newDescription)
320+
{
321+
Interlocked.Exchange(ref _currentDescription, newDescription);
322+
OnDescriptionChanged(oldDescription, newDescription);
323+
}
324+
325+
private void SetDescriptionIfChanged(ServerDescription newDescription)
326+
{
327+
var oldDescription = Interlocked.CompareExchange(ref _currentDescription, null, null);
328+
if (oldDescription.Equals(newDescription))
329+
{
330+
return;
331+
}
332+
333+
SetDescription(oldDescription, newDescription);
334+
}
335+
278336
private void ThrowIfDisposed()
279337
{
280338
if (_state.Value == State.Disposed)

0 commit comments

Comments
 (0)