Skip to content
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

convert ElementTree.ParseError to xsdata.exceptions.ParserError #571

Closed
FirefoxMetzger opened this issue Jul 30, 2021 · 4 comments
Closed

Comments

@FirefoxMetzger
Copy link

More of a proposal than a bug. If you don't have lxml installed or choose to explicitly parse with the built-in ElementTree then ElementTree parse errors are forwarded as-is. I think it could be nice if they could be converted to the xsdata internal ParserError. It makes downstream code cleaner and less specific to xsdata's choice of implementation.

Consider the following snippet

from xsdata.formats.dataclass.context import XmlContext
from xsdata.formats.dataclass.parsers import XmlParser
from dataclasses import dataclass


@dataclass
class Foo:
    class Meta:
        name = "foo"

parser = XmlParser(context=XmlContext())
parser.from_string("<foo>5</foo></foo>", Foo)

at the moment it results in

xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 1, column 13

and handling it (jointly with other parse errors) looks something like

from xsdata.exceptions import ParserError as XSDataParserError
from xml.etree.ElementTree import ParseError as etreeParseError

try: 
    parser.from_string("<foo>5</foo></foo>", Foo)
except etreeParseError:
    pass # handle etree parse error/ malformated XML
except XSDataParserError:
    pass # handle other parser error
@tefra tefra closed this as completed in 81a8cb9 Aug 15, 2021
@tefra
Copy link
Owner

tefra commented Aug 15, 2021

thanks for reporting @FirefoxMetzger,

The xsdata exceptions are due for a refactor, they are a bit messy and not very consistent, but I don't have the energy to deal with this now. I merged a quick patch to wrap all xml syntax errors from all handlers.

Note to self???
The lxml handler is working with recover mode on in order to tackle some xml v1.1 incompatibilities but it also recovers from some xml sytanx errors as well 🤦

@nmrtv
Copy link
Contributor

nmrtv commented Aug 16, 2021

After looking to code such as:

        try:
            parser = sax.make_parser()  # nosec
            parser.setFeature(sax.handler.feature_namespaces, True)
            parser.setContentHandler(self)
            parser.parse(source)
            return self.close()
        except SAXException as e:
            raise SyntaxError(e)

I think it would be better to not eat original exceptions, but wrap them:

        try:
            parser = sax.make_parser()  # nosec
            parser.setFeature(sax.handler.feature_namespaces, True)
            parser.setContentHandler(self)
            parser.parse(source)
            return self.close()
        except SAXException as e:
            raise SyntaxError from e

@nmrtv
Copy link
Contributor

nmrtv commented Aug 16, 2021

Never mind, I see that in such cases, exception chaining occurs automatically. From the docs: Exception chaining happens automatically when an exception is raised inside an except or finally section.

@FirefoxMetzger
Copy link
Author

FirefoxMetzger commented Aug 16, 2021

@neriusmika That is almost correct. The from X part also controls how exceptions are reported if they are not being handled downstream.

If you write something like

try:
    raise ValueError()
except ValueError():
    raise IndexError()

then you will get a stack-trace mentioning During handling of the above exception, another exception occurred. Whereas if you write

try:
    raise ValueError()
except ValueError() as e:
    raise IndexError() from e

your stack trace will get a line mentioning The above exception was the direct cause of the following exception:.

Arguably only relevant if you have complicated logic in your except block and you need to differentiate expected exceptions from unexpected exceptions while debugging and within such a block.

Note also that if you write

try:
    raise ValueError()
except ValueError():
    raise IndexError() from None

you will eat all previous exceptions (here the ValueError) and only raise an IndexError without exception chaining.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants