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

jQuery.parseXML should add detailed information about parse errors #4784

Closed
MatthiasSchmalz opened this issue Aug 31, 2020 · 6 comments · Fixed by #4816
Closed

jQuery.parseXML should add detailed information about parse errors #4784

MatthiasSchmalz opened this issue Aug 31, 2020 · 6 comments · Fixed by #4816
Assignees
@mgol
Labels
Core Has Pull Request
Milestone
3.6.0

Comments

@MatthiasSchmalz
Copy link

Description

If an invalid XML is passed to jQuery.parseXML, currently the thrown parse error only contain the input data but not the error reason.
However the internally created XML document contains the parser errors in the element parsererror as innerText and could be added. This is helpful for debugging errors in XML files

Link to test case

https://jsfiddle.net/o678xuz3/
@mgol
Copy link
Member

mgol commented Aug 31, 2020

Thanks for the report. However, the test case attached doesn't work, it even doesn't include jQuery. Please share a test case where the issue and what you'd expect instead can be reproduced.

@MatthiasSchmalz
Copy link
Author

I'm sorry I have not described the testcase properly.
It is using jQuery which is available implicitely and it uses the jQuery.parseXML. It logs the error message that is raised by the parser to the console.
Additionally it logs the error which the internally used XML parser produces and which should be returned.

@mgol
Copy link
Member

mgol commented Sep 1, 2020

What do you propose us to change? jQuery.parseXML returns the parsed XML and doesn't crash in case of errors. Do you propose that an error we log to the console contained contents of xml.getElementsByTagName( "parsererror" ) as well?

@MatthiasSchmalz
Copy link
Author

I propose to add the xml.getElementsByTagName( "parsererror" )[0].innerText to the text of the thrown Error.

@alayham2 alayham2 mentioned this issue Sep 7, 2020
Closed
@timmywil timmywil reopened this Oct 26, 2020
@timmywil
Copy link
Member

I like this idea. I'd much prefer to either show the parserror content (with provided snippet) or a generic message without trying to add the data to the string.

@timmywil timmywil added Core and removed Needs info labels Oct 26, 2020
@timmywil timmywil added this to the 3.6.0 milestone Oct 26, 2020
@mgol mgol self-assigned this Dec 1, 2020
mgol added a commit to mgol/jquery that referenced this issue Dec 1, 2020
@mgol
Core: Report browser errors in parseXML
2c3fdae 
Fixes jquerygh-4784
@mgol mgol mentioned this issue Dec 1, 2020
Merged
3 tasks
@mgol
Copy link
Member

mgol commented Dec 1, 2020

PR: #4816

@mgol mgol closed this as completed in #4816 Dec 8, 2020
mgol added a commit that referenced this issue Dec 8, 2020
@mgol
Core: Report browser errors in parseXML
8969732 
Fixes gh-4784
Closes gh-4816
mgol added a commit that referenced this issue Dec 8, 2020
@mgol
Core: Report browser errors in parseXML
54d9883 
Fixes gh-4784
Closes gh-4816

(cherry picked from commit 8969732)
@snyk-bot snyk-bot mentioned this issue Apr 11, 2021
Closed
@gguerini gguerini mentioned this issue May 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mgol mgol

Labels
Core Has Pull Request
Milestone
3.6.0
Development

Successfully merging a pull request may close this issue.

Core: Report browser errors in parseXML mgol/jquery
3 participants
@timmywil @mgol @MatthiasSchmalz