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

Catch stackoverflow errors in the highlighter #19836

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

Valentin889
Copy link
Contributor

@Valentin889 Valentin889 commented Mar 1, 2024

StackOverflow Exception on Highlight

Fixes #18596 and fixes #16904.

When there is too much instruction the Sytax Hyghlight can't make the tree and launch a StackOverflow Error

To fix the problem let's catch the Overflow and return the entry string with an error message

the lines that can triggers the error :

val parser = new Parser(source)
val trees = parser.blockStatSeq()
TreeHighlighter.highlight(trees)

To fix it, let's put the try on everything starting from those lines and the catch in the end

case(e): StackOverflowError =>
          in

There is no obvious need of an error message, the command and result will simply not be highlighted so I choose to not put one there.

@mbovel mbovel self-assigned this Mar 6, 2024
@mbovel
Copy link
Member

mbovel commented Mar 6, 2024

There is a failure in SBT tests.

It seems to originate from Scaladoc:

Error:  Caused by: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "java.util.HashMap.get(Object)" is null
Error:  	at com.vladsch.flexmark.html.renderer.HeaderIdGenerator.generateId(HeaderIdGenerator.java:124)
Error:  	at com.vladsch.flexmark.html.renderer.HeaderIdGenerator.getId(HeaderIdGenerator.java:148)
Error:  	at dotty.tools.scaladoc.tasty.comments.markdown.SectionRenderingExtension$SectionHandler$.render(SectionRenderingExtension.scala:38)
Error:  	at dotty.tools.scaladoc.tasty.comments.markdown.SectionRenderingExtension$SectionHandler$.render(SectionRenderingExtension.scala:32)
Error:  	at com.vladsch.flexmark.html.renderer.NodeRenderingHandler.render(NodeRenderingHandler.java:16)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderNode(HtmlRenderer.java:779)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderChildrenNode(HtmlRenderer.java:798)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderChildren(HtmlRenderer.java:790)
Error:  	at com.vladsch.flexmark.html.renderer.CoreNodeRenderer.render(CoreNodeRenderer.java:113)
Error:  	at com.vladsch.flexmark.html.renderer.NodeRenderingHandler.render(NodeRenderingHandler.java:16)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderNode(HtmlRenderer.java:761)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.render(HtmlRenderer.java:683)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:213)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:197)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:230)
Error:  	at dotty.tools.scaladoc.site.TemplateFile.resolveInner(templates.scala:128)
Error:  	at dotty.tools.scaladoc.site.LoadedTemplate.resolveToHtml(LoadedTemplate.scala:54)
Error:  	at dotty.tools.scaladoc.renderers.ResolvedTemplate.<init>(SiteRenderer.scala:18)
Error:  	at dotty.tools.scaladoc.renderers.ResolvedTemplate$.apply(SiteRenderer.scala:17)

Trying to re-run.

@Valentin889 Valentin889 changed the title Vs/stack overflow Catch stackoverflow errors in the highlighter Mar 6, 2024
@hamzaremmal
Copy link
Member

There is a failure in SBT tests.

It seems to originate from Scaladoc:

Error:  Caused by: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "java.util.HashMap.get(Object)" is null
Error:  	at com.vladsch.flexmark.html.renderer.HeaderIdGenerator.generateId(HeaderIdGenerator.java:124)
Error:  	at com.vladsch.flexmark.html.renderer.HeaderIdGenerator.getId(HeaderIdGenerator.java:148)
Error:  	at dotty.tools.scaladoc.tasty.comments.markdown.SectionRenderingExtension$SectionHandler$.render(SectionRenderingExtension.scala:38)
Error:  	at dotty.tools.scaladoc.tasty.comments.markdown.SectionRenderingExtension$SectionHandler$.render(SectionRenderingExtension.scala:32)
Error:  	at com.vladsch.flexmark.html.renderer.NodeRenderingHandler.render(NodeRenderingHandler.java:16)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderNode(HtmlRenderer.java:779)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderChildrenNode(HtmlRenderer.java:798)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderChildren(HtmlRenderer.java:790)
Error:  	at com.vladsch.flexmark.html.renderer.CoreNodeRenderer.render(CoreNodeRenderer.java:113)
Error:  	at com.vladsch.flexmark.html.renderer.NodeRenderingHandler.render(NodeRenderingHandler.java:16)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.renderNode(HtmlRenderer.java:761)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer$MainNodeRenderer.render(HtmlRenderer.java:683)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:213)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:197)
Error:  	at com.vladsch.flexmark.html.HtmlRenderer.render(HtmlRenderer.java:230)
Error:  	at dotty.tools.scaladoc.site.TemplateFile.resolveInner(templates.scala:128)
Error:  	at dotty.tools.scaladoc.site.LoadedTemplate.resolveToHtml(LoadedTemplate.scala:54)
Error:  	at dotty.tools.scaladoc.renderers.ResolvedTemplate.<init>(SiteRenderer.scala:18)
Error:  	at dotty.tools.scaladoc.renderers.ResolvedTemplate$.apply(SiteRenderer.scala:17)

Trying to re-run.

FYI: We have fixed this issue before the repo transfer (See #19884). You might consider rebasing this PR if you have this issue again (they were rare at the time)

@mbovel
Copy link
Member

mbovel commented Mar 11, 2024

Thanks @hamzaremmal.

@Valentin889 can you please rebase this PR on main and see if it passes all tests?

@hamzaremmal
Copy link
Member

Thanks @hamzaremmal.

@Valentin889 can you please rebase this PR on main and see if it passes all tests?

@mbovel PR is already green (you started a second attempt). I don't think the rebasing is necessary.

@mbovel
Copy link
Member

mbovel commented Mar 11, 2024

Ah, great, then we just need an independent review 😄

@sjrd could please have a look?

@mbovel mbovel requested a review from sjrd March 11, 2024 14:04
@SethTisue
Copy link
Member

SethTisue commented Mar 11, 2024

what about #19893 , could this be broadened a bit to improve the situation there too? it's a highlighting crash that isn't an SOE

@mbovel
Copy link
Member

mbovel commented Mar 25, 2024

what about #19893 , could this be broadened a bit to improve the situation there too? it's a highlighting crash that isn't an SOE

I don't know, catching stack overflows was already subject to debate, and catching more errors sound dangerous to me because we probably don't want to silence too many errors automatically. #19893 might be a bug that needs a separate fix, and we wouldn't have gotten a report for it if the error had been caught.

@mbovel mbovel assigned sjrd and unassigned mbovel Mar 25, 2024
Co-authored-by: Sébastien Doeraene <sjrdoeraene@gmail.com>
@mbovel mbovel enabled auto-merge (squash) March 26, 2024 13:04
@mbovel mbovel merged commit ff6ef73 into scala:main Mar 26, 2024
19 checks passed
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
WojciechMazur pushed a commit that referenced this pull request Jul 4, 2024
Co-authored-by: Matt Bovel <matthieu@bovel.net>
Co-authored-by: Sébastien Doeraene <sjrdoeraene@gmail.com>
[Cherry-picked ff6ef73]
WojciechMazur added a commit that referenced this pull request Jul 5, 2024
Backports #19836 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants