Pretty-printing XML validation errors

classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Pretty-printing XML validation errors

Toby Crawley
I've done some work to pretty-print XML validation errors that occur
when parsing (standalone|domain|host).xml, and wanted to get some
feedback on what I have so far to see if:

1) there is interest in seeing this completed
2) the current approach is the best way to integrate with WildFly
3) this same approach could be used to pretty-print issues with other
   xml parsed by WildFly (web.xml, jboss-deployment-structure.xml,
   etc)


# Background

Inspired by error reporting in the Elm language[1] and improvements in
configuration feedback for Clojure tooling[2], I looked at what it
would take to provide better feedback when parsing XML configuration.

My goals were:

* Give users clear feedback that can be used to correct the
  configuration error without the user having to context-switch to
  documentation, and, in most cases, enable the user to quickly
  identify and understand the issue before looking away from the
  validation output, by:

  * Showing (instead of telling) where in the XML the error occurred

  * Providing richer feedback than the native validation error
    provides (detect potential misspellings, provide alternate
    locations, etc)

  * Showing documentation for the element/attribute where possible
    (pulled from the XSD)

* Use what we already produce (XSDs), without having to create
  additional context-specific schema.

I've partially implemented a library (vdx)[3] that, given a validation
error, the source document, and the relevant schemas, generates and
prints a friendly error message. I've also made minimal changes to
WildFly to report validation errors to vdx (see below).


# Examples

Below is some examples of the current startup output from WildFly when
a validation error occurs:


## Detecting a misspelled attribute

13:18:03,798 INFO  [org.jboss.modules] (main) JBoss Modules version 1.5.2.Final
13:18:03,949 INFO  [org.jboss.msc] (main) JBoss MSC version 1.2.6.Final
13:18:04,010 INFO  [org.jboss.as] (MSC service thread 1-6)
WFLYSRV0049: WildFly Core 2.2.0.CR5-SNAPSHOT "Kenny" starting
13:18:04,751 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

 28:         <extension module="org.wildfly.extension.request-controller"/>
 29:         <extension module="org.wildfly.extension.security.manager"/>
 30:         <extension modue="org.wildfly.extension.undertow"/>

                        ^ 'modue' isn't an allowed attribute for the
'extension' element

 31:     </extensions>
 32:     <management>
 33:       <security-realms>

Did you mean 'module'?

================================================================================


13:18:04,753 ERROR [org.jboss.as.server] (Controller Boot Thread)
WFLYSRV0055: Caught exception during boot:
org.jboss.as.controller.persistence.ConfigurationPersistenceException:
WFLYCTL0085: Failed to parse configuration
at org.jboss.as.controller.persistence.XmlConfigurationPersister.load(XmlConfigurationPersister.java:175)
at org.jboss.as.server.ServerService.boot(ServerService.java:357)
at org.jboss.as.controller.AbstractControllerService$1.run(AbstractControllerService.java:299)
at java.lang.Thread.run(Thread.java:745)

13:18:04,754 FATAL [org.jboss.as.server] (Controller Boot Thread)
WFLYSRV0056: Server boot has failed in an unrecoverable manner;
exiting. See previous messages for details.
13:18:04,762 INFO  [org.jboss.as] (MSC service thread 1-3)
WFLYSRV0050: WildFly Core 2.2.0.CR5-SNAPSHOT "Kenny" stopped in 4ms


## Detecting a misplaced attribute

...
14:32:23,842 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

 89:             </console-handler>
 90:             <periodic-rotating-file-handler name="FILE" autoflush="true"
 91:                                             category="WARN">

                                                 ^ 'category' isn't an
allowed attribute for the 'periodic-rotating-file-handler' element

 92:                 <formatter>
 93:                     <named-formatter name="PATTERN"/>
 94:                 </formatter>

'category' is allowed on elements: subsystem > logger, subsystem >
logging-profiles > logging-profile > logger
Did you intend to put it on one of those elements?

================================================================================

...


## Detecting a misplaced element

...

10:54:27,359 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

 81:     </management>
 82:     <profile>
 83:       <extension/>

           ^ 'extension' isn't an allowed element here

 84:         <subsystem xmlns="urn:jboss:domain:logging:3.0">
 85:           <console-handler name="CONSOLE">
 86:             <level name="INFO"/>

'extension' is allowed in elements: domain > extensions, domain >
host-excludes > host-exclude > excluded-extensions, host > extensions,
host > servers > server > extensions, server > extensions
Did you intend to put it in one of those elements?

================================================================================

...


# Changes to WildFly

To support this, I've made some small changes changes to
wildfly-core[4], but anticipate more before this is complete.

The current changes are:

* Modifications to ParseUtils to wrap the XMLStreamExceptions with
  an exception that can convey more context to vdx
* Modifications to XmlConfigurationPersistor to:
  * catch the wrapped exception
  * see if ${jboss.home.dir}/docs/schema/ is available. If so,
    pretty-print the error. If not, throw the wrapped exception
    (which is the behavior before my changes).


# Current issues

* Only the first validation issue is reported, but this is
  unavoidable, since the subsystem parsers throw on the first error
  encountered
* This uses xmlschema-walker from Apache XmlSchema[5], which has a
  couple of bugs that will need to be fixed and released (or forked)
* Only errors reported by throwing Exceptions returned by ParseUtils
  are pretty-printed. Exceptions that come from within STAX reader
  aren't yet handled (for example, a misplaced element in the logging
  parser causes reader.handleAny() to be called[6], which triggers an
  unwrapped exception)
* vdx itself is far from complete[7] - that work is pending the
  outcome of this discussion


# Next steps

As I said above, the first question to answer is: is this an
interesting feature that you would like to see completed?  If so, I'm
willing to continue working on this, and would be happy to discuss
here or on JIRA/HipChat as appropriate.

[1]: http://elm-lang.org/blog/compilers-as-assistants
[2]: http://rigsomelight.com/2016/05/17/good-configuration-feedback-is-essential.html
[3]: https://github.com/projectodd/vdx
[4]: https://github.com/tobias/wildfly-core/commit/b4d03897a6ea1b8c786d983da3b66eab0b3f36b8
[5]: https://ws.apache.org/xmlschema/
[6]: https://github.com/wildfly/wildfly-core/blob/2.x/logging/src/main/java/org/jboss/as/logging/LoggingSubsystemParser_3_0.java#L188
[7]: https://github.com/projectodd/vdx/issues
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry
Thanks for this!

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.

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.

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.

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

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.

On Jul 19, 2016, at 12:29 PM, Toby Crawley <[hidden email]> wrote:

I've done some work to pretty-print XML validation errors that occur
when parsing (standalone|domain|host).xml, and wanted to get some
feedback on what I have so far to see if:

1) there is interest in seeing this completed
2) the current approach is the best way to integrate with WildFly
3) this same approach could be used to pretty-print issues with other
  xml parsed by WildFly (web.xml, jboss-deployment-structure.xml,
  etc)


# Background

Inspired by error reporting in the Elm language[1] and improvements in
configuration feedback for Clojure tooling[2], I looked at what it
would take to provide better feedback when parsing XML configuration.

My goals were:

* Give users clear feedback that can be used to correct the
 configuration error without the user having to context-switch to
 documentation, and, in most cases, enable the user to quickly
 identify and understand the issue before looking away from the
 validation output, by:

 * Showing (instead of telling) where in the XML the error occurred

 * Providing richer feedback than the native validation error
   provides (detect potential misspellings, provide alternate
   locations, etc)

 * Showing documentation for the element/attribute where possible
   (pulled from the XSD)

* Use what we already produce (XSDs), without having to create
 additional context-specific schema.

I've partially implemented a library (vdx)[3] that, given a validation
error, the source document, and the relevant schemas, generates and
prints a friendly error message. I've also made minimal changes to
WildFly to report validation errors to vdx (see below).


# Examples

Below is some examples of the current startup output from WildFly when
a validation error occurs:


## Detecting a misspelled attribute

13:18:03,798 INFO  [org.jboss.modules] (main) JBoss Modules version 1.5.2.Final
13:18:03,949 INFO  [org.jboss.msc] (main) JBoss MSC version 1.2.6.Final
13:18:04,010 INFO  [org.jboss.as] (MSC service thread 1-6)
WFLYSRV0049: WildFly Core 2.2.0.CR5-SNAPSHOT "Kenny" starting
13:18:04,751 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

28:         <extension module="org.wildfly.extension.request-controller"/>
29:         <extension module="org.wildfly.extension.security.manager"/>
30:         <extension modue="org.wildfly.extension.undertow"/>

                       ^ 'modue' isn't an allowed attribute for the
'extension' element

31:     </extensions>
32:     <management>
33:       <security-realms>

Did you mean 'module'?

================================================================================


13:18:04,753 ERROR [org.jboss.as.server] (Controller Boot Thread)
WFLYSRV0055: Caught exception during boot:
org.jboss.as.controller.persistence.ConfigurationPersistenceException:
WFLYCTL0085: Failed to parse configuration
at org.jboss.as.controller.persistence.XmlConfigurationPersister.load(XmlConfigurationPersister.java:175)
at org.jboss.as.server.ServerService.boot(ServerService.java:357)
at org.jboss.as.controller.AbstractControllerService$1.run(AbstractControllerService.java:299)
at java.lang.Thread.run(Thread.java:745)

13:18:04,754 FATAL [org.jboss.as.server] (Controller Boot Thread)
WFLYSRV0056: Server boot has failed in an unrecoverable manner;
exiting. See previous messages for details.
13:18:04,762 INFO  [org.jboss.as] (MSC service thread 1-3)
WFLYSRV0050: WildFly Core 2.2.0.CR5-SNAPSHOT "Kenny" stopped in 4ms


## Detecting a misplaced attribute

...
14:32:23,842 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

89:             </console-handler>
90:             <periodic-rotating-file-handler name="FILE" autoflush="true"
91:                                             category="WARN">

                                                ^ 'category' isn't an
allowed attribute for the 'periodic-rotating-file-handler' element

92:                 <formatter>
93:                     <named-formatter name="PATTERN"/>
94:                 </formatter>

'category' is allowed on elements: subsystem > logger, subsystem >
logging-profiles > logging-profile > logger
Did you intend to put it on one of those elements?

================================================================================

...


## Detecting a misplaced element

...

10:54:27,359 ERROR [org.jboss.as.controller] (Controller Boot Thread)

====================== Validation Error in standalone.xml ======================

81:     </management>
82:     <profile>
83:       <extension/>

          ^ 'extension' isn't an allowed element here

84:         <subsystem xmlns="urn:jboss:domain:logging:3.0">
85:           <console-handler name="CONSOLE">
86:             <level name="INFO"/>

'extension' is allowed in elements: domain > extensions, domain >
host-excludes > host-exclude > excluded-extensions, host > extensions,
host > servers > server > extensions, server > extensions
Did you intend to put it in one of those elements?

================================================================================

...


# Changes to WildFly

To support this, I've made some small changes changes to
wildfly-core[4], but anticipate more before this is complete.

The current changes are:

* Modifications to ParseUtils to wrap the XMLStreamExceptions with
 an exception that can convey more context to vdx
* Modifications to XmlConfigurationPersistor to:
 * catch the wrapped exception
 * see if ${jboss.home.dir}/docs/schema/ is available. If so,
   pretty-print the error. If not, throw the wrapped exception
   (which is the behavior before my changes).


# Current issues

* Only the first validation issue is reported, but this is
 unavoidable, since the subsystem parsers throw on the first error
 encountered
* This uses xmlschema-walker from Apache XmlSchema[5], which has a
 couple of bugs that will need to be fixed and released (or forked)
* Only errors reported by throwing Exceptions returned by ParseUtils
 are pretty-printed. Exceptions that come from within STAX reader
 aren't yet handled (for example, a misplaced element in the logging
 parser causes reader.handleAny() to be called[6], which triggers an
 unwrapped exception)
* vdx itself is far from complete[7] - that work is pending the
 outcome of this discussion


# Next steps

As I said above, the first question to answer is: is this an
interesting feature that you would like to see completed?  If so, I'm
willing to continue working on this, and would be happy to discuss
here or on JIRA/HipChat as appropriate.

[1]: http://elm-lang.org/blog/compilers-as-assistants
[2]: http://rigsomelight.com/2016/05/17/good-configuration-feedback-is-essential.html
[3]: https://github.com/projectodd/vdx
[4]: https://github.com/tobias/wildfly-core/commit/b4d03897a6ea1b8c786d983da3b66eab0b3f36b8
[5]: https://ws.apache.org/xmlschema/
[6]: https://github.com/wildfly/wildfly-core/blob/2.x/logging/src/main/java/org/jboss/as/logging/LoggingSubsystemParser_3_0.java#L188
[7]: https://github.com/projectodd/vdx/issues
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Toby Crawley
On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
<[hidden email]> 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.

> 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.

> 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.

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Heiko W.Rupp
In reply to this post by Toby Crawley
This is awesome.

I was hacking on some subsystem the other day and all I got was
something like "error at char 3162", where I would have already
been happy to know at least the line in standalone.xml.

But this is soo much better :)
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry
In reply to this post by Toby Crawley
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 <[hidden email]> wrote:

On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
<[hidden email]> 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.


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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Toby Crawley
On Tue, Jul 19, 2016 at 4:07 PM, Brian Stansberry
<[hidden email]> wrote:
> 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.

Ah, good point. It shouldn't be difficult to add localized text to the
output, the biggest cost there would be the time to translate the
messages, but I'm not familiar with how i18n is done in our projects.
For the message code, we could print the original exception message at
the bottom or top of the validation block. That would then use the
same code as we provide now, and would provide the same message we use
now with every error. Would that satisfy the message code requirement?

>>
>> On Jul 19, 2016, at 2:38 PM, Toby Crawley <[hidden email]> wrote:
>>
>>> On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
>>> <[hidden email]> 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.)

That's just the first place I saw errors from outside of ParseUtils,
but I haven't yet started playing with attribute values.

>
> 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.
>

If we let these fall back to just pointing out the error location with
the original message, this may be ok (assuming by "the alidators are
meant to produce useful exception messages" you mean the messages
produced by the AttributeDefinitions).

<snip>

>>> 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.
>

I'm only using commons-lang for a Levenshtein distance implementation
- that could certainly be pulled in, and we could drop the 284k for
commons-lang. It certainly would be nice to keep core under 17MB
though.

- Toby

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Tomaž Cerar-2
Most of my concerns about this ware already raised by Brian (l18n, extra size of distro, inconsistencies between subsystems, ...)

Also on top of this, having something like that would glorify configuration of server via XML,
which is something we are trying to discourage  in favor of CLI / mgmt api.
Another problem is also that it relies on XSD which are not always 100% correct, which should be fixed (but that is another problem)

There are also ideas about implementing non xml backend for storage of our configuration.
type of storage for the backend shouldn't really matter as users should be interacting via API / CLI not by manually modifying the file.

--
tomaz


On Wed, Jul 20, 2016 at 1:28 AM, Toby Crawley <[hidden email]> wrote:
On Tue, Jul 19, 2016 at 4:07 PM, Brian Stansberry
<[hidden email]> wrote:
> 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.

Ah, good point. It shouldn't be difficult to add localized text to the
output, the biggest cost there would be the time to translate the
messages, but I'm not familiar with how i18n is done in our projects.
For the message code, we could print the original exception message at
the bottom or top of the validation block. That would then use the
same code as we provide now, and would provide the same message we use
now with every error. Would that satisfy the message code requirement?

>>
>> On Jul 19, 2016, at 2:38 PM, Toby Crawley <[hidden email]> wrote:
>>
>>> On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
>>> <[hidden email]> 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.)

That's just the first place I saw errors from outside of ParseUtils,
but I haven't yet started playing with attribute values.

>
> 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.
>

If we let these fall back to just pointing out the error location with
the original message, this may be ok (assuming by "the alidators are
meant to produce useful exception messages" you mean the messages
produced by the AttributeDefinitions).

<snip>

>>> 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.
>

I'm only using commons-lang for a Levenshtein distance implementation
- that could certainly be pulled in, and we could drop the 284k for
commons-lang. It certainly would be nice to keep core under 17MB
though.

- Toby

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Aleksandar Kostadinov
Tomaž Cerar wrote on 07/20/16 16:38:
> Most of my concerns about this ware already raised by Brian (l18n, extra
> size of distro, inconsistencies between subsystems, ...)
>
> Also on top of this, having something like that would glorify
> configuration of server via XML,
> which is something we are trying to discourage  in favor of CLI / mgmt api.

Excuse my ignorance if this is already taken care of. But last time I
looked, using whatever management API was not very nice for container
runtime. i.e. configuration changed through API was not persisted.
I am wondering if this is already fixed somehow or is at least somebody
considering it as an important use case (I think it should).
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry

On Jul 20, 2016, at 8:59 AM, Aleksandar Kostadinov <[hidden email]> wrote:

Tomaž Cerar wrote on 07/20/16 16:38:
Most of my concerns about this ware already raised by Brian (l18n, extra
size of distro, inconsistencies between subsystems, ...)

Also on top of this, having something like that would glorify
configuration of server via XML,
which is something we are trying to discourage  in favor of CLI / mgmt api.

Excuse my ignorance if this is already taken care of. But last time I
looked, using whatever management API was not very nice for container
runtime. i.e. configuration changed through API was not persisted.
I am wondering if this is already fixed somehow or is at least somebody
considering it as an important use case (I think it should).

Persisting server configuration changes made via the supported management APIs back to the config file has been done in AS 7.0 and later. Its a fundamental requirement.

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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Aleksandar Kostadinov
Brian Stansberry wrote on 07/20/16 20:23:

>
>> On Jul 20, 2016, at 8:59 AM, Aleksandar Kostadinov
>> <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Tomaž Cerar wrote on 07/20/16 16:38:
>>> Most of my concerns about this ware already raised by Brian (l18n, extra
>>> size of distro, inconsistencies between subsystems, ...)
>>>
>>> Also on top of this, having something like that would glorify
>>> configuration of server via XML,
>>> which is something we are trying to discourage  in favor of CLI /
>>> mgmt api.
>>
>> Excuse my ignorance if this is already taken care of. But last time I
>> looked, using whatever management API was not very nice for container
>> runtime. i.e. configuration changed through API was not persisted.
>> I am wondering if this is already fixed somehow or is at least somebody
>> considering it as an important use case (I think it should).
>
> Persisting server configuration changes made via the supported
> management APIs back to the config file has been done in AS 7.0 and
> later. Its a fundamental requirement.

I should have explained better. The config is saved on container. But
container being stateless loses changes on restart. At least for
OpenShift v2 that has been an issue.

Shame on me I didn't look at OpenShift v3 integration. I'm not sure how
it is handled there. Perhaps I should have done my homework before
hijacking the thread. Sorry about that. Hopefully I can check wildfly on
v3 next week.
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Toby Crawley
In reply to this post by Tomaž Cerar-2
On Wed, Jul 20, 2016 at 9:38 AM, Tomaž Cerar <[hidden email]> wrote:
> Also on top of this, having something like that would glorify configuration
> of server via XML,
> which is something we are trying to discourage  in favor of CLI / mgmt api.

I suspect I'm not the typical WildFly user, but I never use the
CLI/API to configure or deploy. I much prefer to edit configuration by
hand, and don't like requiring the server to be up to configure
it. And Aleksandar has raised the issue about immutable deployments -
if you use the CLI for every config change, you have to spin up the
server before or as part of your build process in order to generate a
configuration file to put into your container. But all that is
orthogonal to the pretty-printing discussion.

With regards to glorifying editing the XML - I disagree. If a user
already prefers editing the XML over using the CLI, this makes things
more pleasant for them. If this makes editing the XML more appealing
than using the CLI to people that would normally prefer the CLI, then
that would imply there is room for improvement in the feedback that
that CLI gives.

> Another problem is also that it relies on XSD which are not always 100%
> correct, which should be fixed (but that is another problem)
>

I agree, if the schemas aren't correct, we should fix that.

> There are also ideas about implementing non xml backend for storage of our
> configuration.
> type of storage for the backend shouldn't really matter as users should be
> interacting via API / CLI not by manually modifying the file.

If another backend is being considered, I hope it too will be
user-editable. And if it is, a similar feedback tool could be applied
to it.

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

jtgreene
Administrator
In reply to this post by Aleksandar Kostadinov

> On Jul 20, 2016, at 5:49 PM, Aleksandar Kostadinov <[hidden email]> wrote:
>
> Brian Stansberry wrote on 07/20/16 20:23:
>>
>>> On Jul 20, 2016, at 8:59 AM, Aleksandar Kostadinov
>>> <[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> Tomaž Cerar wrote on 07/20/16 16:38:
>>>> Most of my concerns about this ware already raised by Brian (l18n, extra
>>>> size of distro, inconsistencies between subsystems, ...)
>>>>
>>>> Also on top of this, having something like that would glorify
>>>> configuration of server via XML,
>>>> which is something we are trying to discourage  in favor of CLI /
>>>> mgmt api.
>>>
>>> Excuse my ignorance if this is already taken care of. But last time I
>>> looked, using whatever management API was not very nice for container
>>> runtime. i.e. configuration changed through API was not persisted.
>>> I am wondering if this is already fixed somehow or is at least somebody
>>> considering it as an important use case (I think it should).
>>
>> Persisting server configuration changes made via the supported
>> management APIs back to the config file has been done in AS 7.0 and
>> later. Its a fundamental requirement.
>
> I should have explained better. The config is saved on container. But
> container being stateless loses changes on restart. At least for
> OpenShift v2 that has been an issue.
>
> Shame on me I didn't look at OpenShift v3 integration. I'm not sure how
> it is handled there. Perhaps I should have done my homework before
> hijacking the thread. Sorry about that. Hopefully I can check wildfly on
> v3 next week.

Currently, in the case of OS/docker you would typically stage your config in advance, so you would use the CLI to author the config which is then included in your image.

Note that since 10 we have offline CLI, which allows you to use management ops without a running server.

--
Jason T. Greene
WildFly Lead / JBoss EAP Platform Architect
JBoss, a division of Red Hat


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

jtgreene
Administrator
In reply to this post by Toby Crawley

> On Jul 21, 2016, at 1:02 PM, Toby Crawley <[hidden email]> wrote:
>
>>
>> Another problem is also that it relies on XSD which are not always 100%
>> correct, which should be fixed (but that is another problem)
>>
>
> I agree, if the schemas aren't correct, we should fix that.

This is a bit of tangent, but the problem is that system properties would require us to have very complex schemas that would make it difficult for authorship tools to aid the user. We essentially end up with unions for everything with a union on an expression syntax pattern which allows almost anything.

--
Jason T. Greene
WildFly Lead / JBoss EAP Platform Architect
JBoss, a division of Red Hat


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry
In reply to this post by Aleksandar Kostadinov

On Jul 20, 2016, at 5:49 PM, Aleksandar Kostadinov <[hidden email]> wrote:

Brian Stansberry wrote on 07/20/16 20:23:

On Jul 20, 2016, at 8:59 AM, Aleksandar Kostadinov
<[hidden email] <[hidden email]>> wrote:

Tomaž Cerar wrote on 07/20/16 16:38:
Most of my concerns about this ware already raised by Brian (l18n, extra
size of distro, inconsistencies between subsystems, ...)

Also on top of this, having something like that would glorify
configuration of server via XML,
which is something we are trying to discourage  in favor of CLI /
mgmt api.

Excuse my ignorance if this is already taken care of. But last time I
looked, using whatever management API was not very nice for container
runtime. i.e. configuration changed through API was not persisted.
I am wondering if this is already fixed somehow or is at least somebody
considering it as an important use case (I think it should).

Persisting server configuration changes made via the supported
management APIs back to the config file has been done in AS 7.0 and
later. Its a fundamental requirement.

I should have explained better. The config is saved on container. But container being stateless loses changes on restart. At least for OpenShift v2 that has been an issue.


Thanks; I figured I was missing something. :)

Shame on me I didn't look at OpenShift v3 integration. I'm not sure how it is handled there. Perhaps I should have done my homework before hijacking the thread. Sorry about that. Hopefully I can check wildfly on v3 next week.

Jason has already replied with what I was going to say about ways to prepare the config other than editing xml. I’ll just follow up a bit on what he said by mentioning that I presented at a meeting of the folks who prepare middleware images for use on OpenShift and showed them the offline CLI, which they were interested in since it eases scripting of configuration. I don’t know how much it’s been adapted since then though.

I recognize that end users are going to want to edit xml, and that’s fine, people can use techniques that best fit their preferences. I think it’s mistaken though for *tools* to be based on manipulating our xml configs. We work hard to ensure long term compatibility in our management APIs, but we have no such guarantees about our xml formats. We encourage normal users to prefer our management APIs over xml for the same reason, but an individual having to check the xsd again if our format changes is a smaller problem than a tool having to be rewritten.

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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Tomaž Cerar-2
In reply to this post by Toby Crawley

On Thu, Jul 21, 2016 at 8:02 PM, Toby Crawley <[hidden email]> wrote:
if you use the CLI for every config change, you have to spin up the
server before or as part of your build process in order to generate a
configuration file to put into your container.


Well that is not really needed anymore, we do have embeded server in CLI.
so you can just have CLI script that starts with embed-server (or embed-host-controller for domain)
and than all the cli commands you need.
than all you need is ./jboss-cli.sh --file path-to-config.cli

This will start embedded server in admin mode so you can configure it without fully starting it.

--
tomaz


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Aleksandar Kostadinov
In reply to this post by Brian Stansberry
Brian Stansberry wrote on 07/21/16 22:55:

>
>> On Jul 20, 2016, at 5:49 PM, Aleksandar Kostadinov
>> <[hidden email] <mailto:[hidden email]>> wrote:
>>
>> Brian Stansberry wrote on 07/20/16 20:23:
>>>
>>>> On Jul 20, 2016, at 8:59 AM, Aleksandar Kostadinov
>>>> <[hidden email] <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>> Tomaž Cerar wrote on 07/20/16 16:38:
>>>>> Most of my concerns about this ware already raised by Brian (l18n,
>>>>> extra
>>>>> size of distro, inconsistencies between subsystems, ...)
>>>>>
>>>>> Also on top of this, having something like that would glorify
>>>>> configuration of server via XML,
>>>>> which is something we are trying to discourage  in favor of CLI /
>>>>> mgmt api.
>>>>
>>>> Excuse my ignorance if this is already taken care of. But last time I
>>>> looked, using whatever management API was not very nice for container
>>>> runtime. i.e. configuration changed through API was not persisted.
>>>> I am wondering if this is already fixed somehow or is at least somebody
>>>> considering it as an important use case (I think it should).
>>>
>>> Persisting server configuration changes made via the supported
>>> management APIs back to the config file has been done in AS 7.0 and
>>> later. Its a fundamental requirement.
>>
>> I should have explained better. The config is saved on container. But
>> container being stateless loses changes on restart. At least for
>> OpenShift v2 that has been an issue.
>>
>
> Thanks; I figured I was missing something. :)
>
>> Shame on me I didn't look at OpenShift v3 integration. I'm not sure
>> how it is handled there. Perhaps I should have done my homework before
>> hijacking the thread. Sorry about that. Hopefully I can check wildfly
>> on v3 next week.
>
> Jason has already replied with what I was going to say about ways to
> prepare the config other than editing xml. I’ll just follow up a bit on
> what he said by mentioning that I presented at a meeting of the folks
> who prepare middleware images for use on OpenShift and showed them the
> offline CLI, which they were interested in since it eases scripting of
> configuration. I don’t know how much it’s been adapted since then though.
>
> I recognize that end users are going to want to edit xml, and that’s
> fine, people can use techniques that best fit their preferences. I think
> it’s mistaken though for *tools* to be based on manipulating our xml
> configs. We work hard to ensure long term compatibility in our
> management APIs, but we have no such guarantees about our xml formats.
> We encourage normal users to prefer our management APIs over xml for the
> same reason, but an individual having to check the xsd again if our
> format changes is a smaller problem than a tool having to be rewritten.

Offline cli is a nice improvement. The OpenShift Wildfly image might be
configured to read cli commands from a file in the app repo to apply
configuration before launching server.

This would not give the ability to reconfigure cluster using management
console though. Maybe that's not important for container use cases. I
don't know. Just would be nice somebody to figure out what an *ultimate*
Wildfly UX in OpenShift means so that core dev team can implement
necessary features to get there. Just retrofitting to the cloud has less
chances to attract (I'm not saying we do that).
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Aleksandar Kostadinov
In reply to this post by Aleksandar Kostadinov
Heiko W.Rupp wrote on 07/27/16 14:57:

> On 21 Jul 2016, at 0:49, Aleksandar Kostadinov wrote:
>>
>> I should have explained better. The config is saved on container. But
>> container being stateless loses changes on restart. At least for
>
> Yeah, that is a bit of an issue. One needs to mount the whole config
> dir to an external volume(*) and then on container creation populate it
> with a default configuration.
> Or externalize the changes of the standard config and apply it each
> time at container start (docker run, not docker start).

OpenShift does support persistent volumes. Thinking about auto-scale I'm
not sure that would help a lot though.

I'm wondering whether domain mode can help - having domain node(s) with
persistent storage configuration for management. Then the worker nodes
would read config from domain.

I suspect though most people would not want to waste resources for
non-worker nodes and would expect to have things working OOB in some
simple manner. Perhaps wildfly may support committing configuration
changes in whatever format to some external system (git, file server,
whatever), then read it from there.
e.g. if wildfly can commit any and all configuration changes to git, an
OpenShift deployment config can trigger redeploy based on the noticed
changes automatically (or user can trigger manually).

btw it would help if Wildfly records only changes from defaults (not all
configuration files). In this way migrating between versions and
changing defaults might work much smoother.
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry
There’s lots of discussion around this general topic at https://issues.jboss.org/browse/WFCORE-433.

On Jul 27, 2016, at 12:17 PM, Aleksandar Kostadinov <[hidden email]> wrote:

Heiko W.Rupp wrote on 07/27/16 14:57:
On 21 Jul 2016, at 0:49, Aleksandar Kostadinov wrote:

I should have explained better. The config is saved on container. But
container being stateless loses changes on restart. At least for

Yeah, that is a bit of an issue. One needs to mount the whole config
dir to an external volume(*) and then on container creation populate it
with a default configuration.
Or externalize the changes of the standard config and apply it each
time at container start (docker run, not docker start).

OpenShift does support persistent volumes. Thinking about auto-scale I'm not sure that would help a lot though.

I'm wondering whether domain mode can help - having domain node(s) with persistent storage configuration for management. Then the worker nodes would read config from domain.

I suspect though most people would not want to waste resources for non-worker nodes and would expect to have things working OOB in some simple manner. Perhaps wildfly may support committing configuration changes in whatever format to some external system (git, file server, whatever), then read it from there.
e.g. if wildfly can commit any and all configuration changes to git, an OpenShift deployment config can trigger redeploy based on the noticed changes automatically (or user can trigger manually).

btw it would help if Wildfly records only changes from defaults (not all configuration files). In this way migrating between versions and changing defaults might work much smoother.

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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Toby Crawley
In reply to this post by Toby Crawley
I've done some more work on VDX, and have addressed most of the
technical concerns about it from this thread, namely:

* I18N - VDX is now i18n ready, but currently just has a default
  message set in english.
* Displaying the default message with code - the original message is
  now displayed as part of the output, see
  https://gist.github.com/tobias/ec2846a13b6ff656d8e47cbc85071cb1 for
  an example.
* Handling errors that aren't created by ParseUtils - For any
  XMLStreamExceptions that don't get wrapped by ParseUtils, VDX now
  shows that error the best it can. This still doesn't handle errors
  that aren't XMLStreamExceptions, but is progress. This should also
  handle errors thrown from AttributeDefinitions, since they are
  XMLStreamExceptions.
https://gist.github.com/tobias/551e712892b9326096ecc80bb78bc132
  is an example of handling a non-wrapped exception thrown by the
  reader.
* Size of dependencies - VDX now has no dependencies, and is currently
  61K. Removing the dependencies on Apache XMLSchema and Apache
  Commons Lang reduces the total size by 552K.

Existing issues:

* I18N - though it is set up for i18n, it doesn't yet have any
  translations. How many/what languages do we support for WildFly
  currently? What is the process for getting translations done?
* non-XMLStreamException errors - it's currently possible to trigger
  errors that aren't XMLStreamExceptions when parsing
  standalone.xml. A couple of examples are:

  - adding a duplicate extension under server > extensions causes an
    IllegalStateException that occurs when trying to generate the
    XMLStreamException
    (https://gist.github.com/tobias/59d155afe0c88e268b83cb75734353eb)
  - Creating a file-handler with no attributes under server >
    management > audit-log > handlers results in an error about there
    being no add operation at address [], with no other context
    (https://gist.github.com/tobias/34b7d7791e20148004d883238adf16ac). If
    you add a name attribute, you get a different, more helpful
    management operation error
    (https://gist.github.com/tobias/6d47539aa40de90ec45cfaaec83ea15e),
    but neither error points to the xml configuration as the problem.

  Nothing can really be done with VDX to help with these issues, but I
  would be happy to provide PR's to fix them as I find them if there
  is interest.

The question now is - should I keep working on this? Is there enough
interest in this for me to continue it to the point where it can be
used by wildfly-core?

Is there interest in seeing this applied to xml that is provided as
part of a deployment (web.xml, jboss-web.xml,
jboss-deployment-structure.xml, etc) in addition to (or instead of
standalone.xml)? I haven't yet looked at what that would take.

For reference, the source for VDX is available at
https://github.com/projectodd/vdx, and my changes so far to
wildfly-core are in this commit:
https://github.com/tobias/wildfly-core/commit/2654406bfcfed7cbf255db819f094749487e2d10

- Toby

On Tue, Jul 19, 2016 at 7:28 PM, Toby Crawley <[hidden email]> wrote:

> On Tue, Jul 19, 2016 at 4:07 PM, Brian Stansberry
> <[hidden email]> wrote:
>> 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.
>
> Ah, good point. It shouldn't be difficult to add localized text to the
> output, the biggest cost there would be the time to translate the
> messages, but I'm not familiar with how i18n is done in our projects.
> For the message code, we could print the original exception message at
> the bottom or top of the validation block. That would then use the
> same code as we provide now, and would provide the same message we use
> now with every error. Would that satisfy the message code requirement?
>
>>>
>>> On Jul 19, 2016, at 2:38 PM, Toby Crawley <[hidden email]> wrote:
>>>
>>>> On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
>>>> <[hidden email]> 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.)
>
> That's just the first place I saw errors from outside of ParseUtils,
> but I haven't yet started playing with attribute values.
>
>>
>> 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.
>>
>
> If we let these fall back to just pointing out the error location with
> the original message, this may be ok (assuming by "the alidators are
> meant to produce useful exception messages" you mean the messages
> produced by the AttributeDefinitions).
>
> <snip>
>
>>>> 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.
>>
>
> I'm only using commons-lang for a Levenshtein distance implementation
> - that could certainly be pulled in, and we could drop the 284k for
> commons-lang. It certainly would be nice to keep core under 17MB
> though.
>
> - Toby

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Pretty-printing XML validation errors

Brian Stansberry
Hi Toby,

Sorry for the delay. People I wanted to talk to have been traveling and on vacation etc.

I’ll reply in-line. Bottom line is it looks like you’ve made good progress and this seems worth pursuing. Thanks for digging into it!

> On Aug 9, 2016, at 3:13 PM, Toby Crawley <[hidden email]> wrote:
>
> I've done some more work on VDX, and have addressed most of the
> technical concerns about it from this thread, namely:
>
> * I18N - VDX is now i18n ready, but currently just has a default
>  message set in english.
> * Displaying the default message with code - the original message is
>  now displayed as part of the output, see
>  https://gist.github.com/tobias/ec2846a13b6ff656d8e47cbc85071cb1 for
>  an example.
> * Handling errors that aren't created by ParseUtils - For any
>  XMLStreamExceptions that don't get wrapped by ParseUtils, VDX now
>  shows that error the best it can. This still doesn't handle errors
>  that aren't XMLStreamExceptions, but is progress. This should also
>  handle errors thrown from AttributeDefinitions, since they are
>  XMLStreamExceptions.

What does the failure message look like when AttributeDefinition rejects an attribute? I want to make sure this is helping, not obscuring.

A simple test would be to change the ‘port’ attribute in a socket-binding element to -1.

> https://gist.github.com/tobias/551e712892b9326096ecc80bb78bc132
>  is an example of handling a non-wrapped exception thrown by the
>  reader.
> * Size of dependencies - VDX now has no dependencies, and is currently
>  61K. Removing the dependencies on Apache XMLSchema and Apache
>  Commons Lang reduces the total size by 552K.
>

Great!

> Existing issues:
>
> * I18N - though it is set up for i18n, it doesn't yet have any
>  translations. How many/what languages do we support for WildFly
>  currently? What is the process for getting translations done?

For WildFly itself, English is fine. But for things that end up in EAP (which this would) we need the ability for the Red Hat localization team to do translations and get the properties files into VDX source. We do this via a tool called Zanata, available at https://translate.jboss.org, with integration via a maven plugin. We can help you get this set up once things are near done. The biggest thing is that your code is i18n which means it can be done.

> * non-XMLStreamException errors - it's currently possible to trigger
>  errors that aren't XMLStreamExceptions when parsing
>  standalone.xml. A couple of examples are:
>
>  - adding a duplicate extension under server > extensions causes an
>    IllegalStateException that occurs when trying to generate the
>    XMLStreamException
>    (https://gist.github.com/tobias/59d155afe0c88e268b83cb75734353eb)

This one looks to be the parser author (aka me) being lazy and incorrectly using a util method instead of throwing a more custom exception.

I filed https://issues.jboss.org/browse/WFCORE-1717 for this and listed you as the reporter. Thanks!

>  - Creating a file-handler with no attributes under server >
>    management > audit-log > handlers results in an error about there
>    being no add operation at address [], with no other context
>    (https://gist.github.com/tobias/34b7d7791e20148004d883238adf16ac). If
>    you add a name attribute, you get a different, more helpful
>    management operation error
>    (https://gist.github.com/tobias/6d47539aa40de90ec45cfaaec83ea15e),
>    but neither error points to the xml configuration as the problem.
>

This sounds like the parser not doing validation that it could, and deferring to the management op handling.

>  Nothing can really be done with VDX to help with these issues, but I
>  would be happy to provide PR's to fix them as I find them if there
>  is interest.
>

Parser improvements are welcome. Somewhat. ;) This gets back a bit to something I said in my first post in this thread:

"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.”

So if we get some simple, no risk patches, great, thank you! I just don’t want to start having people insist that we beef up the level of validation in our parsers so we can get pretty reports. But then not contributing the parser improvements.

WFCORE-1717 is different; there the parser is just wrong.

> The question now is - should I keep working on this? Is there enough
> interest in this for me to continue it to the point where it can be
> used by wildfly-core?
>
> Is there interest in seeing this applied to xml that is provided as
> part of a deployment (web.xml, jboss-web.xml,
> jboss-deployment-structure.xml, etc) in addition to (or instead of
> standalone.xml)? I haven't yet looked at what that would take.
>
> For reference, the source for VDX is available at
> https://github.com/projectodd/vdx, and my changes so far to
> wildfly-core are in this commit:
> https://github.com/tobias/wildfly-core/commit/2654406bfcfed7cbf255db819f094749487e2d10
>
> - Toby
>
> On Tue, Jul 19, 2016 at 7:28 PM, Toby Crawley <[hidden email]> wrote:
>> On Tue, Jul 19, 2016 at 4:07 PM, Brian Stansberry
>> <[hidden email]> wrote:
>>> 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.
>>
>> Ah, good point. It shouldn't be difficult to add localized text to the
>> output, the biggest cost there would be the time to translate the
>> messages, but I'm not familiar with how i18n is done in our projects.
>> For the message code, we could print the original exception message at
>> the bottom or top of the validation block. That would then use the
>> same code as we provide now, and would provide the same message we use
>> now with every error. Would that satisfy the message code requirement?
>>
>>>>
>>>> On Jul 19, 2016, at 2:38 PM, Toby Crawley <[hidden email]> wrote:
>>>>
>>>>> On Tue, Jul 19, 2016 at 3:15 PM, Brian Stansberry
>>>>> <[hidden email]> 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.)
>>
>> That's just the first place I saw errors from outside of ParseUtils,
>> but I haven't yet started playing with attribute values.
>>
>>>
>>> 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.
>>>
>>
>> If we let these fall back to just pointing out the error location with
>> the original message, this may be ok (assuming by "the alidators are
>> meant to produce useful exception messages" you mean the messages
>> produced by the AttributeDefinitions).
>>
>> <snip>
>>
>>>>> 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.
>>>
>>
>> I'm only using commons-lang for a Levenshtein distance implementation
>> - that could certainly be pulled in, and we could drop the 284k for
>> commons-lang. It certainly would be nice to keep core under 17MB
>> though.
>>
>> - Toby

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




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
12