Skip to content
This repository has been archived by the owner on Apr 22, 2022. It is now read-only.

Handle exceptions during event mapping. #113

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Handle exceptions during event mapping. #113

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bed7214
Handle exceptions during event mapping.
cbsmith Dec 4, 2015
a08f09f
Moved listener.incomingRequest & doProcess out.
cbsmith Dec 8, 2015
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
17 changes: 14 additions & 3 deletions src/main/java/io/divolte/server/IncomingRequestProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.apache.avro.generic.GenericRecord;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.Marker;
import org.slf4j.MarkerFactory;

import io.divolte.record.DefaultEventRecord;
import io.divolte.server.config.ValidatedConfiguration;
Expand All @@ -46,6 +48,8 @@

@ParametersAreNonnullByDefault
public final class IncomingRequestProcessor implements ItemProcessor<DivolteEvent> {
public static final String INVALID_DATA_MARKER = "INVALID";
public static final Marker invalid = MarkerFactory.getMarker(INVALID_DATA_MARKER);
private static final Logger logger = LoggerFactory.getLogger(IncomingRequestProcessor.class);

public static final AttachmentKey<Boolean> DUPLICATE_EVENT_KEY = AttachmentKey.create(Boolean.class);
Expand Down Expand Up @@ -150,14 +154,21 @@ public ProcessingDirective process(final DivolteEvent event) {

if (!duplicate || keepDuplicates) {
final GenericRecord avroRecord = mapper.newRecordFromExchange(event);
final AvroRecordBuffer avroBuffer = AvroRecordBuffer.fromRecord(

try {
final AvroRecordBuffer avroBuffer = AvroRecordBuffer.fromRecord(
event.partyCookie,
event.sessionCookie,
event.requestStartTime,
event.clientUtcOffset,
avroRecord);
listener.incomingRequest(event, avroBuffer, avroRecord);
doProcess(avroBuffer);
} catch (final RuntimeException e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The listener call and the doProcess should go outside of the try-catch block, if we're guarding against mapping errors here.

Copy link
Author

Choose a reason for hiding this comment

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

If you look at the stack trace from #110, you will see it actually propagated up from fromRecord. I guess it wasn't strictly an error in the mapping logic, but rather the generic schema being employed. Either way, I think it is safer to include it.

Copy link
Author

Choose a reason for hiding this comment

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

But to clarify, listener.incomingRequest & doProcess are both inside the try/catch block.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I would consider the fromRecord to be part of the mapping (going from event to Avro) and doProcess(...) is part of handling the (then valid) avro record for enqueueing. Any error at that point would really be a bug and not a mapping error.

The listener is really just there for testing purposes (there's certain classes in the HTTP implementation that we cannot mock, so we use this path instead). Running Divolte normally shouldn't use it. If there is an exception in the listener, it's clearly a programming error.

Copy link
Author

Choose a reason for hiding this comment

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

Oh geez. I misread your feedback... I read "inside" where you said "outside". Okay. I'll update the pull.

logger.warn(invalid, "Error processing event {} from party {} in session {}: {}",
event, event.partyCookie, event.sessionCookie, avroRecord, e);
}

listener.incomingRequest(event, avroBuffer, avroRecord);
doProcess(avroBuffer);
}
}

Expand Down