Provide document id upon ConversionException

[StWitte]

Hi,
Sometimes after a Java model change from, for example, LocalDateTime to LocalDate there are existing documents that must be read into a LocalDate but are in Elastic with the DateTime format.
This fails with exception:

org.springframework.data.elasticsearch.core.convert.ConversionException: Unable to convert value '2017-11-17T23:00:00.000+0000' to java.time.LocalDate for property 'date'
	at org.springframework.data.elasticsearch.core.convert.TemporalPropertyValueConverter.read(TemporalPropertyValueConverter.java:60)
	at org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter$Reader.convertOnRead(MappingElasticsearchConverter.java:524)
	at org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter$Reader.propertyConverterRead(MappingElasticsearchConverter.java:518)
	at org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter$Reader.readValue(MappingElasticsearchConverter.java:432)
	at 

This is of course fine but we don't get the document Id in the error context so it is quite a challenge to find out which document is involved. And when streaming or iterating repository results we don't get any of the batch that the one failing document is in.

While I'm on Java 8, spring-data-elasticsearch-4.4.14, spring-boot-starter-data-elasticsearch-2.7.14 I see in the latest code in:

spring-data-elasticsearch/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java

Line 337 in c96423d

R result = readProperties(targetEntity, instance, valueProvider);

			ElasticsearchPropertyValueProvider valueProvider = new ElasticsearchPropertyValueProvider(accessor, evaluator);
			R result = readProperties(targetEntity, instance, valueProvider);

			if (source instanceof Document document) {
				if (document.hasId()) {
					ElasticsearchPersistentProperty idProperty = targetEntity.getIdProperty();
					PersistentPropertyAccessor<R> propertyAccessor = new ConvertingPropertyAccessor<>(
							targetEntity.getPropertyAccessor(result), conversionService);
					// Only deal with String because ES generated Ids are strings !
					if (idProperty != null && idProperty.isReadable() && idProperty.getType().isAssignableFrom(String.class)) {
						propertyAccessor.setProperty(idProperty, document.getId());
					}
				}

				if (document.hasVersion()) {
					long version = document.getVersion();
					ElasticsearchPersistentProperty versionProperty = targetEntity.getVersionProperty();
					// Only deal with Long because ES versions are longs !
					if (versionProperty != null && versionProperty.getType().isAssignableFrom(Long.class)) {
						// check that a version was actually returned in the response, -1 would indicate that
						// a search didn't request the version ids in the response, which would be an issue
						Assert.isTrue(version != -1, "Version in response is -1");
						targetEntity.getPropertyAccessor(result).setProperty(versionProperty, version);
					}
				}

That the document Id is being read after the readProperties where the ConversionException is thrown.
I would suggest that the ConversionException is catched here and, if possible, is enriched with the document id. This would be a great help for troubleshooting and removing failing documents.

[sothawo]

That's doable. But we should introduce something like a MappingException that has the underlying ConversionException as it's cause and not modify an exception thrown somehwere else.

[StWitte]

Thanks, this would be a great help in troubleshooting.