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

Add resource path to the exception message when parsing XML mapper failed #1172

Merged
merged 3 commits into from Jan 5, 2018

Conversation

Projects
None yet
2 participants
@iMouseWu
Contributor

iMouseWu commented Dec 29, 2017

when xmlConfigBuilder parse xml file error,the stack of the error will not identify which file it is.
I think printing error detail in threadLocal may be more friendly for developers

I can not agree more that putting the error message in the core @harawata
see another PR

iMouseWu added some commits Dec 29, 2017

@@ -117,7 +117,7 @@ private void configurationElement(XNode context) {
sqlElement(context.evalNodes("/mapper/sql"));
buildStatementFromContext(context.evalNodes("select|insert|update|delete"));
} catch (Exception e) {
throw new BuilderException("Error parsing Mapper XML. Cause: " + e, e);
throw new BuilderException("Error parsing Mapper XML." + ErrorContext.instance() + " Cause: " + e, e);

This comment has been minimized.

@harawata

harawata Dec 29, 2017

Member

It's better not to use ErrorContext.
The mapper file location is in this.resource, so please use it to build the message.

This comment has been minimized.

@iMouseWu

iMouseWu Jan 5, 2018

Contributor

It is better to use resource than ErrorContext In this case,because nobody knows how much infomation the context contains .Also it is the weakness to use context( For example
Chain of Responsibility )
But I am curious about the code design of ErrorContext .I'm puzzled about using ErrorContext ,I can find putting the infomation in it,but I cannot find where to get the infomation
Thank You.

This comment has been minimized.

@harawata

harawata Jan 5, 2018

Member

It is used in ExceptionFactory#wrapException(), I think.

This comment has been minimized.

@iMouseWu

iMouseWu Jan 8, 2018

Contributor

Thank you for your prompt answer

@harawata harawata changed the title from print error detail when xmlConfigBuilder parse error to Add XML file path to the exception message when parsing XML mapper failed Jan 5, 2018

@harawata harawata changed the title from Add XML file path to the exception message when parsing XML mapper failed to Add resource path to the exception message when parsing XML mapper failed Jan 5, 2018

@harawata harawata merged commit f9fabc3 into mybatis:master Jan 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@harawata harawata self-assigned this Jan 5, 2018

@harawata harawata added the enhancement label Jan 5, 2018

@harawata harawata added this to the 3.4.6 milestone Jan 5, 2018

@harawata

This comment has been minimized.

Member

harawata commented Jan 5, 2018

Thank you, @iMouseWu !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment