Skip to content

Commit f0644cf

Browse files
Catch practices: avoid losing catched exception information. (#1555)
* Catch practices: avoid losing catched exception information. All catches have been reviewed for avoiding losing original exception information: * When another exception is raised, it should embed the original one as an inner exception. * Otherwise when re-thrown, it should be done without altering its original stack-trace. Preferred way is of course to use "throw;" in the catch clause without specifying the exception, but otherwise ReflectHelper provides a way to preserve the stack-trace. * Otherwise it should be transmitted to the logger. Some cases still do not follow this pattern for avoiding being too noisy, like "Try" utilities, failure in closing an already failed object, some "implemented by exception" features, ...
1 parent 8d2b123 commit f0644cf

24 files changed

+109
-70
lines changed

src/NHibernate/Async/Cache/StandardQueryCache.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ public async Task<IList> GetAsync(QueryKey key, ICacheAssembler[] returnTypes, b
115115
result.Add(await (TypeHelper.AssembleAsync((object[])cacheable[i], returnTypes, session, null, cancellationToken)).ConfigureAwait(false));
116116
}
117117
}
118-
catch (UnresolvableObjectException)
118+
catch (UnresolvableObjectException ex)
119119
{
120120
if (isNaturalKeyLookup)
121121
{
122122
//TODO: not really completely correct, since
123123
// the UnresolvableObjectException could occur while resolving
124124
// associations, leaving the PC in an inconsistent state
125-
Log.Debug("could not reassemble cached result set");
125+
Log.Debug(ex, "could not reassemble cached result set");
126126
await (_queryCache.RemoveAsync(key, cancellationToken)).ConfigureAwait(false);
127127
return null;
128128
}

src/NHibernate/Async/Event/Default/DefaultMergeEventListener.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,11 @@ private async Task<object> MergeTransientEntityAsync(object entity, string entit
263263

264264
if (((EventCache)copyCache).IsOperatedOn(propertyFromEntity))
265265
{
266-
log.Info("property '{0}.{1}' from original entity is in copyCache and is in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
266+
log.Info(ex, "property '{0}.{1}' from original entity is in copyCache and is in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
267267
}
268268
else
269269
{
270-
log.Info("property '{0}.{1}' from original entity is in copyCache and is not in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
270+
log.Info(ex, "property '{0}.{1}' from original entity is in copyCache and is not in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
271271
}
272272

273273
// continue...; we'll find out if it ends up not getting saved later

src/NHibernate/Async/Impl/SessionFactoryImpl.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System;
1212
using System.Collections.Concurrent;
1313
using System.Collections.Generic;
14+
using System.Collections.ObjectModel;
1415
using System.Data.Common;
1516
using System.Linq;
1617
using System.Runtime.Serialization;

src/NHibernate/Cache/StandardQueryCache.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,14 @@ public IList Get(QueryKey key, ICacheAssembler[] returnTypes, bool isNaturalKeyL
133133
result.Add(TypeHelper.Assemble((object[])cacheable[i], returnTypes, session, null));
134134
}
135135
}
136-
catch (UnresolvableObjectException)
136+
catch (UnresolvableObjectException ex)
137137
{
138138
if (isNaturalKeyLookup)
139139
{
140140
//TODO: not really completely correct, since
141141
// the UnresolvableObjectException could occur while resolving
142142
// associations, leaving the PC in an inconsistent state
143-
Log.Debug("could not reassemble cached result set");
143+
Log.Debug(ex, "could not reassemble cached result set");
144144
_queryCache.Remove(key);
145145
return null;
146146
}

src/NHibernate/Cfg/SettingsFactory.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ public Settings BuildSettings(IDictionary<string, string> properties)
6262
{
6363
sqlExceptionConverter = SQLExceptionConverterFactory.BuildSQLExceptionConverter(dialect, properties);
6464
}
65-
catch (HibernateException)
65+
catch (HibernateException he)
6666
{
67-
log.Warn("Error building SQLExceptionConverter; using minimal converter");
67+
log.Warn(he, "Error building SQLExceptionConverter; using minimal converter");
6868
sqlExceptionConverter = SQLExceptionConverterFactory.BuildMinimalSQLExceptionConverter();
6969
}
7070
settings.SqlExceptionConverter = sqlExceptionConverter;

src/NHibernate/Cfg/XmlHbmBinding/ClassBinder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,9 +371,9 @@ protected void BindAnyMeta(IAnyMapping anyMapping, Any model)
371371
string entityName = GetClassName(metaValue.@class, mappings);
372372
values[value] = entityName;
373373
}
374-
catch (InvalidCastException)
374+
catch (InvalidCastException ice)
375375
{
376-
throw new MappingException("meta-type was not an IDiscriminatorType: " + metaType.Name);
376+
throw new MappingException("meta-type was not an IDiscriminatorType: " + metaType.Name, ice);
377377
}
378378
catch (HibernateException he)
379379
{

src/NHibernate/Collection/AbstractPersistentCollection.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ public object Current
6161
{
6262
return enclosingInstance.operationQueue[position].AddedInstance;
6363
}
64-
catch (IndexOutOfRangeException)
64+
catch (IndexOutOfRangeException ex)
6565
{
66-
throw new InvalidOperationException();
66+
throw new InvalidOperationException(
67+
"MoveNext as not been called or its last call has yielded false (meaning the enumerator is beyond the end of the enumeration).",
68+
ex);
6769
}
6870
}
6971
}

src/NHibernate/Engine/StatefulPersistenceContext.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ void IDeserializationCallback.OnDeserialization(object sender)
14391439
}
14401440
catch (HibernateException he)
14411441
{
1442-
throw new InvalidOperationException(he.Message);
1442+
throw new InvalidOperationException(he.Message, he);
14431443
}
14441444
}
14451445

@@ -1467,7 +1467,7 @@ void IDeserializationCallback.OnDeserialization(object sender)
14671467
}
14681468
catch (MappingException me)
14691469
{
1470-
throw new InvalidOperationException(me.Message);
1470+
throw new InvalidOperationException(me.Message, me);
14711471
}
14721472
}
14731473
}

src/NHibernate/Event/Default/DefaultMergeEventListener.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,11 @@ private object MergeTransientEntity(object entity, string entityName, object req
266266

267267
if (((EventCache)copyCache).IsOperatedOn(propertyFromEntity))
268268
{
269-
log.Info("property '{0}.{1}' from original entity is in copyCache and is in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
269+
log.Info(ex, "property '{0}.{1}' from original entity is in copyCache and is in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
270270
}
271271
else
272272
{
273-
log.Info("property '{0}.{1}' from original entity is in copyCache and is not in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
273+
log.Info(ex, "property '{0}.{1}' from original entity is in copyCache and is not in the process of being merged; {1} =[{2}]", copyEntry.EntityName, propertyName, propertyFromEntity);
274274
}
275275

276276
// continue...; we'll find out if it ends up not getting saved later

src/NHibernate/Hql/Ast/ANTLR/HqlParser.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using Antlr.Runtime;
33

44
using NHibernate.Hql.Ast.ANTLR.Tree;
5+
using NHibernate.Util;
56
using IToken = Antlr.Runtime.IToken;
67
using RecognitionException = Antlr.Runtime.RecognitionException;
78

@@ -418,6 +419,7 @@ public IASTNode HandleIdentifierError(IToken token, RecognitionException ex)
418419
}
419420

420421
// Otherwise, handle the error normally.
422+
ReflectHelper.PreserveStackTrace(ex);
421423
throw ex;
422424
}
423425
}

0 commit comments

Comments
 (0)