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.

Sign up for GitHub

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

Jump to bottom

PAGE reader: throw error msg #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

PAGE reader: throw error msg #16

wants to merge 2 commits into from

Conversation

bertsky
Copy link
Contributor

@bertsky bertsky commented May 25, 2023

fixes #12

example (from PageViewer):

org.primaresearch.io.UnsupportedFormatVersionException: cvc-id.1: There is no ID/IDREF binding for IDREF 'OCR-D-CLIP_Ansiedlung_Korotschin_UZS_Sign_22a_0000_region0039'.
	at org.primaresearch.dla.page.io.xml.XmlPageReader.read(XmlPageReader.java:150)
	at org.primaresearch.dla.page.io.xml.PageXmlInputOutput.readPage(PageXmlInputOutput.java:224)
	at org.primaresearch.page.viewer.dla.XmlDocumentLayoutLoader.doRun(XmlDocumentLayoutLoader.java:49)
	at org.primaresearch.page.viewer.extra.Task.run(Task.java:49)
	at org.primaresearch.page.viewer.extra.Task$TaskThread.run(Task.java:105)

@bertsky
Copy link
Contributor Author

bertsky commented May 25, 2023

Because they are not Throwable directly, I had to wrap the IOErrors in the message content of a UnsupportedFormatVersionException. Let me know if you have a better design.

@bertsky bertsky mentioned this pull request Jun 9, 2023
Open
@bertsky bertsky mentioned this pull request Feb 7, 2024
Open
stweil
stweil reviewed Feb 7, 2024
View reviewed changes
java/PrimaDla/src/org/primaresearch/dla/page/io/xml/XmlPageReader.java
StringBuilder sb = new StringBuilder();
for (IOError error : getErrors())
sb.append(error.getMessage());
throw new UnsupportedFormatVersionException(sb.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new code will not only report the error, but also throw an error instead of returning null. Can that cause new issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a library, yes, throwing an exception is the design of choice. Downstream applications can decide how to act on that. So the viewer or the converter might want to present an error message about invalid input. But for the moment, the stack trace will suffice – remember the user had nothing up until now. (I remember hours-long hunts on failed PAGE files for sublime non-syntactic errors like missing @conf or dangling @regionRef.)

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

@stweil stweil stweil left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

reader/validation: throw informative exception
2 participants
@bertsky @stweil