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

Error when including a template that extends another #385

Open
Arnaz87 opened this issue Jan 26, 2019 · 16 comments
Open

Error when including a template that extends another #385

Arnaz87 opened this issue Jan 26, 2019 · 16 comments
Labels

Comments

@Arnaz87
Copy link

Arnaz87 commented Jan 26, 2019

Test project showcasing the error: https://github.com/Arnaz87/tera-test

And the traceback:

thread 'main' panicked at 'internal error: entered unreachable code: render_node -> unexpected node: Extends(WS { left: false, right: false }, "parent-holder.txt")', /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/processor.rs:812:18
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:59
             at src/libstd/panicking.rs:211
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:491
   5: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:398
   6: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:353
   7: tera::renderer::processor::Processor::render_node
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/processor.rs:812
   8: tera::renderer::processor::Processor::render_body
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/processor.rs:150
   9: tera::renderer::processor::Processor::render_node
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/processor.rs:804
  10: tera::renderer::processor::Processor::render
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/processor.rs:858
  11: tera::renderer::Renderer::render
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/renderer/mod.rs:54
  12: tera::tera::Tera::render
             at /home/arnaud/.cargo/git/checkouts/tera-ae1ac1f255426cfc/68dfeb5/src/tera.rs:309
  13: tera_test::main
             at src/main.rs:7
  14: std::rt::lang_start::{{closure}}
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/rt.rs:74
  15: std::panicking::try::do_call
             at src/libstd/rt.rs:59
             at src/libstd/panicking.rs:310
  16: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:102
  17: std::rt::lang_start_internal
             at src/libstd/panicking.rs:289
             at src/libstd/panic.rs:398
             at src/libstd/rt.rs:58
  18: std::rt::lang_start
             at /rustc/9fda7c2237db910e41d6a712e9a2139b352e558b/src/libstd/rt.rs:74
  19: main
  20: __libc_start_main
  21: _start
@Keats
Copy link
Owner

Keats commented Jan 26, 2019

Ah perfect, I guess that's a reproduction of #370

Not sure what the behaviour should be though, probably an error message instead of a panic

@Keats Keats added the bug label Jan 27, 2019
@Keats
Copy link
Owner

Keats commented Jan 27, 2019

The main issue with allowing extends in include is that you can end up with situations where the included template is in the inheritance tree of the template including it so you end up with an infinite loop.

In practice include in Tera should almost never be used anyway, macros are preferred.

@Arnaz87
Copy link
Author

Arnaz87 commented Jan 28, 2019

Isn't it better to explicitly detect infinite loops?

How can macros be used in this case?

@Arnaz87
Copy link
Author

Arnaz87 commented Jan 28, 2019

In our project we need potentially complex documents in other bigger, also complex documents, I don't know macros very well but I don't know if they are powerful enough to cover that use case.

@naturallymitchell
Copy link

With a configurable recursion depth limit in Tera and app-level prevention of insane template structures, I think we'd handle the complexity well from both ends.

@Keats
Copy link
Owner

Keats commented Jan 28, 2019

I never use include myself, it's only there because it sometimes is easier to include some bits than using a macro.

Do you have an example of complex include? Macros are like functions returning text so you can do everything allowed in Tera aside from inheritance

@naturallymitchell
Copy link

the software as a whole is fairly complex. would you run it if installation was simple and easy enough to quickly try this out?

@Keats
Copy link
Owner

Keats commented Jan 28, 2019

Is it not possible to get a sample included file?
I was hesitating removing include completely` for v1 actually so it would be useful to have some info on how it is used.
If it's only possible through the software, I'll have a look later if you put the URL

@naturallymitchell
Copy link

naturallymitchell commented Jan 28, 2019

I'm pretty sure this is our use case that causes the bug: https://github.com/lighttouch-themes/subscription-menu/blob/master/layouts/base-example.html

but we use include a lot in other places

@Keats
Copy link
Owner

Keats commented Jan 28, 2019

That indeed uses include a lot, way more than I thought it would be used :o
It doesn't seem like that repo has included templates extending a base layout though, they look like regular includes

@Arnaz87
Copy link
Author

Arnaz87 commented Jan 29, 2019

It doesn't uses inheritance in includes yet precisely because it's broken, hence the issue

@Keats
Copy link
Owner

Keats commented Jan 29, 2019

I am personally not planning to implement that feature, is one of you interested in doing it?

@naturallymitchell
Copy link

is one of you interested in doing it?

yes, Arnaud said he will code it

if you have any tips ahead, that's cool

@Keats
Copy link
Owner

Keats commented Jan 31, 2019

I think that's not going to be trivial as allowing extends means you have to render the include as actual template with the current context (eg an include in a loop).
Maybe creating a new Processor and just calling render on that would work but then you need to account for recursion and potentially other unforeseen complications.

@Arnaz87
Copy link
Author

Arnaz87 commented Feb 1, 2019

foundpatterns@e4a4ef9

Is there anything wrong with this approach? It was doing something with the call stack and pushing an include frame that I removed, is that important? I think the included template then doesn't have access to locals declared in the including template, when previously it had, is that right?

It's still missing the recursion check, but I want to know if it's going the right way.

No tests are failing with these changes (Except all the error cases, wtf)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants