apple / pkl

A configuration as code language with rich validation and tooling.
https://pkl-lang.org
Apache License 2.0
10.4k stars 280 forks source link

Crash help text has mangled URL #61

Open Edward-Knight opened 9 months ago

Edward-Knight commented 9 months ago

The help text output when a crash happens has a URL to help submit a bug report, however this URL is mangled due to a missing newline:

$ echo "a { b = a }" | pkl eval - 2>&1 | head
An unexpected error has occurred. Would you mind filing a bug report?
Cmd+Double-click the link below to open an issue.
Please copy and paste the entire error output into the issue's description, provided you can share it.

https://github.com/apple/pkl/issues/newjava.lang.StackOverflowError

–– Pkl Error ––
Stack overflow

Pkl 0.25.1 (macOS 14.1, native)

The URL is https://github.com/apple/pkl/issues/newjava.lang.StackOverflowError instead of https://github.com/apple/pkl/issues/new.

This seems to be caused by a missing newline at the end of this paragraph:

https://github.com/apple/pkl/blob/6eb3d20b472f4ae53595edf13b3499520c075b76/pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionRenderer.java#L57-L62


I can open a separate issue for the stack overflow if needed - seems sometimes this is caught, e.g.:

$ echo "output { value = output }" | pkl eval - 2>&1
–– Pkl Error ––
A stack overflow occurred.

┌─ 4001 repetitions of:
│ 106 | text = renderer.renderDocument(value)
│              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│ at pkl.base#Module.output.text (https://github.com/apple/pkl/blob/0.25.1/stdlib/base.pkl#L106)
└─
Edward-Knight commented 9 months ago

Or actually, looking at the code further, is the URL missing a ?body=? So it should be https://github.com/apple/pkl/issues/new?body=java.lang.StackOverflowError?

holzensp commented 9 months ago

Fixed by PR #73

Edward-Knight commented 9 months ago

Although #73 will ensure the URL is always valid, I don't think it's a full fix. The next line tries to URL encode the error, presumably so it can be put in the title or body of the GitHub issue

is the URL missing a ?body=? So it should be https://github.com/apple/pkl/issues/new?body=java.lang.StackOverflowError?

Edward-Knight commented 9 months ago

Also should I open a separate issue about the stack overflow or is the current behaviour expected?

holzensp commented 9 months ago

The stack overflow is expected, although arguably it's missing a stack frame (cc @stackoverflow and not just for the ironically appropriate username). In Pkl, a = a is the simplest way to cause a stack overflow. That's essentially what you're doing here, because in output { value = output }, the inner output "ties a loop" with the outer output.

Re-opening to follow up about the query parts of the URL (which may have been rolled into the URL encoding fix)