-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
log full stack traces on error #37
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,6 @@ public class PublishHook implements CRUDHook, LightblueFactoryAware { | |
|
||
public static final String HOOK_NAME = "publishHook"; | ||
public static final String ENTITY_NAME = "esbEvents"; | ||
public static final String ERR_MISSING_ID = HOOK_NAME + ":MissingID"; | ||
|
||
private LightblueFactory lightblueFactory; | ||
private static transient JsonNodeFactory factory = JsonNodeFactory.withExactBigDecimals(true); | ||
|
@@ -102,18 +101,13 @@ public void processHook(EntityMetadata entityMetadata, HookConfiguration hookCon | |
if (eventConfiguration.getRootIdentityFields() != null && eventConfiguration.getRootIdentityFields().size() > 0) { | ||
event.addRootIdentities(getRootIdentities(event.getIdentity(), eventConfiguration.getRootIdentityFields())); | ||
} | ||
try { | ||
insert(ENTITY_NAME, event); | ||
} catch (ClassNotFoundException | IllegalAccessException | InvocationTargetException | NoSuchMethodException | ||
| InstantiationException | IOException e) { | ||
LOGGER.error("Unexpected error", e); | ||
} | ||
|
||
insert(ENTITY_NAME, event); | ||
} | ||
} | ||
} | ||
} catch (IllegalArgumentException | JSONException e) { | ||
LOGGER.error(e.toString()); | ||
throw Error.get(HOOK_NAME + ":UnexpectedException", e); | ||
} | ||
} | ||
} | ||
|
@@ -133,25 +127,29 @@ private List<Identity> getRootIdentities(List<Identity> identities, List<String> | |
return rootIdentities; | ||
} | ||
|
||
private void insert(String entityName, Object entity) throws ClassNotFoundException, IllegalAccessException, InvocationTargetException, IOException, | ||
NoSuchMethodException, InstantiationException { | ||
private void insert(String entityName, Object entity) { | ||
ObjectNode jsonNode = new ObjectNode(JsonNodeFactory.instance); | ||
jsonNode.put("entity", entityName); | ||
ArrayNode data = jsonNode.putArray("data"); | ||
data.add(new ObjectMapper().valueToTree(entity)); | ||
|
||
InsertionRequest ireq = InsertionRequest.fromJson(jsonNode); | ||
Response r = lightblueFactory.getMediator().insert(ireq); | ||
Response r; | ||
try { | ||
r = lightblueFactory.getMediator().insert(ireq); | ||
} catch (ClassNotFoundException | IllegalAccessException | InvocationTargetException | NoSuchMethodException | InstantiationException | IOException e) { | ||
throw Error.get(HOOK_NAME + ":MediatorException", e); | ||
} | ||
if (!r.getErrors().isEmpty()) { | ||
// TODO Better Handle Exception | ||
for (Error e : r.getErrors()) { | ||
LOGGER.error(e.toString()); | ||
LOGGER.error("Lightblue Error", e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A more descriptive error message would be useful here. Same for the next one, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can use a different logger, and send the error logs to a separate logfile? |
||
} | ||
throw Error.get(HOOK_NAME + ":Error", "Errors while attempting to insert esb events."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with Naveen. Log errors, don't throw. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bserdar does the hook framework protect against implementations throwing On Wed, Sep 30, 2015 at 9:27 AM, Burak Serdar notifications@github.com
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mediator catches all exceptions at the end, and anything that's not handled On Wed, Sep 30, 2015 at 7:28 AM, Naveen Malik notifications@github.com
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created lightblue-platform/lightblue-core#486 On Wed, Sep 30, 2015 at 9:30 AM, Burak Serdar notifications@github.com
|
||
} else if (!r.getDataErrors().isEmpty()) { | ||
// TODO Better Handle Exception | ||
for (DataError e : r.getDataErrors()) { | ||
LOGGER.error(e.toString()); | ||
LOGGER.error("Lightblue Data Error", e); | ||
} | ||
throw Error.get(HOOK_NAME + ":DataError", "Eata errors while attempting to insert esb events."); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means there is a configuration error. You can throw a runtime exception for this one.