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

Properly handle errors originating from included files when compileDebug is enabled #3269

Merged
merged 3 commits into from Feb 28, 2021

Conversation

@tmont
Copy link
Contributor

@tmont tmont commented Jun 18, 2020

When calling render/compile with { compileDebug: true }, and an error was thrown from (or due to) a local, the str parameter in pug_rethrow was a toJSON()'d Buffer, and the rethrow code tried to split it like a string.

str would be something like:

{
  type: 'Buffer',
  data: [ /* a bunch of numbers */ ]
}

which is what happens when you do something like Buffer.from('foo').toJSON().

I couldn't really figure out where or how it happens, but I just made the rethrow function gracefully handle those serialized Buffer objects properly.

I wrote a test that failed (basically just calls an undefined function from an included file):
pug-error-reporting

and then fixed it in this PR.

@rollingversions
Copy link

@rollingversions rollingversions bot commented Jun 18, 2020

pug (3.0.0 → 3.0.1)

Bug Fixes

  • Serialize Buffers to strings when storing sources for use with compileDebug: true

pug-runtime (3.0.0 → 3.0.1)

Bug Fixes

  • Properly handle non-string values when rethrowing errors

Packages With No Changes

The following packages have no user facing changes, so won't be released:

  • pug-attrs
  • pug-code-gen
  • pug-error
  • pug-filters
  • pug-lexer
  • pug-linker
  • pug-load
  • pug-parser
  • pug-strip-comments
  • pug-walk

Edit changelogs

If compileDebug is enabled, a Buffer would be serialized
and then pug-runtime would try to split() it assuming
it would always be a string.
@tmont tmont force-pushed the tmont:str-split-fix branch from 1b980ff to 7ea9f36 Jun 18, 2020
@ForbesLindesay
Copy link
Member

@ForbesLindesay ForbesLindesay commented Jul 2, 2020

Thanks for this. The fix will need to be done where we create the debug_sources object (

debug_sources[filename] = contents;
)

I think the best bet would be to move that code into the lex method (

lex: function(str, options) {
) so we can store the actual string, instead of a Buffer. The secondary advantage is that we only need debug_sources for files that we lex so it would produce a smaller output via not having any non-pug includes.

P.S. the filename should be available as options.filename in the lex function.

@tmont tmont force-pushed the tmont:str-split-fix branch from 399b9e4 to c86a762 Dec 7, 2020
@tmont
Copy link
Contributor Author

@tmont tmont commented Dec 7, 2020

@ForbesLindesay Finally had time to revisit this. I also got tired of manually modifying the sources to apply this patch every time my template code has an error just so I can see what it was. :)

I moved the Buffer check code to the read function in main pug package and out of pug-runtime (although I left in some better error reporting in the case of weird stuff happening when trying to manage the strings, but that should only happen in internal code I think).

@ForbesLindesay ForbesLindesay merged commit d4b7f60 into pugjs:master Feb 28, 2021
4 checks passed
4 checks passed
@github-actions
test (10.x)
Details
@github-actions
test (12.x)
Details
@github-actions
test (14.x)
Details
@rollingversions
RollingVersions releasing multiple packages
Details
This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants