Comments in-line, except for something I just thought of.

All exception and log messages produced will likely need to follow WildFly’s/EAP's i18n standards. Message prefixed with a code, text produced in a way in an i18n manner with a reasonable way to get localized text into the software.

On Jul 19, 2016, at 2:38 PM, Toby Crawley <tcrawley@redhat.com> wrote:

On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
<brian.stansberry@redhat.com> wrote:
The only big concern I have about this is that we’ll get this behavior for
some failures but not all. And I don’t want to go down the path of trying to
force every parser to work in a manner such that we consistently get this.


I haven't looked at it too deeply, but it may be straightforward to
alter staxmapper to allow providing an exception generator that would
allow catching more of the cases that the parsers miss.


I’m not sure how big of a problem staxmapper-thrown exceptions are. (I haven’t really thought.)

What I was thinking more about when I wrote my previous post was parsers not using ParseUtils, or sometimes not using it. 

Also, a lot of XmlStreamException cases are generated from implementations of org.jboss.as.controller.AttributeDefinition, e.g.

https://github.com/wildfly/wildfly-core/blob/master/controller/src/main/java/org/jboss/as/controller/SimpleAttributeDefinition.java#L140

Parsers are encouraged to invoke methods on AttributeDefinition to validate attribute values. Perhaps though those are better left alone, as the validators are meant to produce useful exception messages.

Personally I think it’s ok to have this for only some failures. Others may
disagree though and start filing bug reports, leading to demands that we fix
said “bugs”, leading to a shift of resource away from other tasks.

My instinct is it’s worth it though. I’m curious what others think.


Adding more to the WildFly team's plate is a concern of mine as well.
I'm willing to do the work to finish this feature, and to help out
with any reported issues related to it in the future, but there would
still be some increase in the team's workload from it.

I think the path you’ve followed is a good way to get a lot of benefit
without being overly intrusive.

A medium sized concern is this has to be robust. It can’t be producing
misleading messages, as that’s worse than simply pointing to the line/col of
where the mistake was.


Agreed - there needs to be some confidence level that if the tool
can't meet for an error, it falls back to just pointing to the
line/col, printing the original message. I fact, that may be good
enough for the failures that bubble out of staxmapper as well.

A minor concern is how big the added dependencies are. (I don’t know.) We
want to keep WildFly Core small in footprint.


Right now, the only dependencies vdx (31k) has are commons-lang (which
is already a module in WildFly, but not core-feature-pack),
xmlschema-walker (100k), and xmlschema-core (168k). For the rest of
the work, I don't currently see needing any more dependencies.

Thanks. So about 583K including 284K for commons-lang. The current wildly-core-dist-3.0.0.Alpha3.zip is about 16.9MB, so this is fairly substantial.

Re: "Only the first validation issue is reported, but this is unavoidable,
since the subsystem parsers throw on the first error encountered” — I’m not
bothered by that at all. We’re booting a server, not validating a document.
If people are producing documents riddled with errors there are other tools
to use to help with that.

Makes sense, thanks.

-- 
Brian Stansberry
Manager, Senior Principal Software Engineer
JBoss by Red Hat