[BUG] View: If errors are returned by the ViewRenderer - do not output a "half template"

[Dexus]

Describe the bug
I am currently using the latest version from the Master Branch. If I use a template with Jet that uses variables that are not present, an error is returned, but the template is already sent to the browser as far as it has come, this should be prevented.

To Reproduce
We have tried with and without iris.WithResetOnFireErrorCode but without ctx.Record()

app.Get("/bug-test", func(ctx iris.Context) {
	ctx.ViewData("demo", "123")
	if err := ctx.View("bugtest.jet"); err != nil {
		ctx.StopWithText(iris.StatusInternalServerError, "Templates not rendered!")
	}
})

Expected behavior
Only the StopWithText should returned or the OnAnyErrorCode should be triggered. Not the half rendered Template.

I would expect, if a ViewEngine throws an error, that there will be no normal output, even if the template system has already sent it to the output.

An alternative would be to document it more clearly, in connection with the ViewEngines.

[Dexus]

Another point is that if you use ctx.Record() and iris.Compression then you can't send errors because the compression already uses a recorder and therefore the actual interception with ctx.Record() is impossible.

[kataras]

@Dexus This has to do with the CloudyKit/jet template, not iris' view engine. There is a relative issue at: CloudyKit/jet#110. And the code, I can edit and push a PR to add an option to fix that is there: https://github.com/CloudyKit/jet/blob/e02d8822d3974184cce9f3c6b5b7a7a27389f408/eval.go#L534 and: https://github.com/CloudyKit/jet/blob/cc9978f14830540788c1b68bd4fe02dd6cb9a9e9/template.go#L309. However I am not sure if this is possible there, it's a runtime error by processing blocks, so the previous are rendered, the result is written one by one, it is not cached until all blocks are parsed. Even if I push a fix there, it will require adding a buffer and parse the template, and if success then write to the main writer (render to client), but you can already do that with ctx.Record() and WithResetOnFireErrorCode. So I am not sure if we must do something about that or just add a ctx.Record() -> ctx.View -> err!=nil { ctx.StatusCode(errCode) or ctx.StopWithXXX to a jet example. What's your opinion?

[kataras]

Another point is that if you use ctx.Record() and iris.Compression then you can't send errors because the compression already uses a recorder and therefore the actual interception with ctx.Record() is impossible.

Yes, but it supposed that the following fixes it:

iris/core/router/handler.go

Lines 461 to 488 in 75d7fe7

	if ctx.ResponseWriter().Written() > 0 {
	return
	}
	statusCode := ctx.GetStatusCode() // the response's cached one.
	if ctx.Application().ConfigurationReadOnly().GetResetOnFireErrorCode() /* could be an argument too but we must not break the method */ {
	// if we can reset the body, probably manual call of `Application.FireErrorCode`.
	if w, ok := ctx.IsRecording(); ok {
	if statusCodeSuccessful(w.StatusCode()) { // if not an error status code
	w.WriteHeader(statusCode) // then set it manually here, otherwise it should be set via ctx.StatusCode(...)
	}
	// reset if previous content and it's recorder, keep the status code.
	w.ClearHeaders()
	w.ResetBody()
	} else if w, ok := ctx.ResponseWriter().(*context.CompressResponseWriter); ok {
	// reset and disable the gzip in order to be an expected form of http error result
	w.Disabled = true
	}
	} else {
	// check if a body already set (the error response is handled by the handler itself,
	// see `Context.EndRequest`)
	if w, ok := ctx.IsRecording(); ok {
	if len(w.Body()) > 0 {
	return
	}
	}
	}

Did you try the iris.WithResetOnFireErrorCode setting? It's added after this issue (which describes the opposite) and it's described on the HISTORY's Breaking Changes section.

[Dexus]

Hi @kataras

Another point is that if you use ctx.Record() and iris.Compression then you can't send errors because the compression already uses a recorder and therefore the actual interception with ctx.Record() is impossible.

here my example:

package main

import (
	"bytes"
	"encoding/base64"
	"fmt"
	"reflect"

	"github.com/kataras/iris/v12"
	"github.com/kataras/iris/v12/middleware/logger"
	"github.com/kataras/iris/v12/middleware/recover"
	"github.com/kataras/iris/v12/view"
)

var tmpl *view.JetEngine

// GetView return the view.JetEngine for later use
func GetView() *view.JetEngine {
	return tmpl
}

// RegisterView for Iris and all View Helper
func RegisterView(app *iris.Application) *view.JetEngine {
	tmpl = iris.Jet("views", ".jet") // <--
	tmpl.Reload(true)                // remove in production.
	tmpl.AddFunc("base64", func(a view.JetArguments) reflect.Value {
		a.RequireNumOfArguments("base64", 1, 1)

		buffer := bytes.NewBuffer(nil)
		fmt.Fprint(buffer, a.Get(0))

		return reflect.ValueOf(base64.URLEncoding.EncodeToString(buffer.Bytes()))
	})
	app.RegisterView(tmpl) // <--

	return tmpl
}

func main() {
	app := iris.New()
	app.Logger().SetLevel("info") //"debug"
	// Optionally, add two built'n handlers
	// that can recover from any http-relative panics
	// and log the requests to the terminal.
	app.Use(recover.New())
	app.Use(logger.New())

	RegisterView(app)

	// BUG: The OnAnyErrorCode will work if this is commented
	// but as soon as you uncomment it, the webpage get rendered and 
	// delivered unfinished to the client

	//app.Use(iris.Compression)

	app.OnAnyErrorCode(func(ctx iris.Context) {
		err := ctx.GetErr()
		if err != nil {
			ctx.Text(err.Error())
			return
		}
		ctx.Text(iris.StatusText(ctx.GetStatusCode()))
	})

	// Method:   GET
	// Resource: http://localhost:8080/plans
	app.Get("/plans", func(ctx iris.Context) {
		ctx.Record()
		ctx.ViewData("demo", "123")
		if err := ctx.View("pages/plans.jet"); err != nil {
			ctx.SetErr(err)
			ctx.StopWithStatus(iris.StatusInternalServerError)
		}
	})

	app.Run(
		iris.Addr(":8080"),
		iris.WithoutServerError(iris.ErrServerClosed),
		iris.WithCharset("utf-8"),
		iris.WithOptimizations,
		iris.WithResetOnFireErrorCode,
		iris.WithPostMaxMemory(1024*1024*3),
	)
}

Template:

<html>
<head><title>broken page</title></head>
<body>
<p>some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... some text... </p>
{{ .demo }}<br>
<p>some text that never rendered even not the error that should trigger the code below</p>
<pre><code>
app.OnAnyErrorCode(func(ctx iris.Context) {
		err := ctx.GetErr()
		if err != nil {
			ctx.Text(err.Error())
			return
		}
		ctx.Text(iris.StatusText(ctx.GetStatusCode()))
	})
</code></pre>
</body>
</html>

ctx.Record() and WithResetOnFireErrorCode works, but only if you not have app.Use(iris.Compression) in use.

@Dexus This has to do with the CloudyKit/jet template, not iris' view engine. ..... Even if I push a fix there, it will require adding a buffer and parse the template, and if success then write to the main writer (render to client), but you can already do that with ctx.Record() and WithResetOnFireErrorCode. So I am not sure if we must do something about that or just add a ctx.Record() -> ctx.View -> err!=nil { ctx.StatusCode(errCode) or ctx.StopWithXXX to a jet example. What's your opinion?

This works, but still causes problems if a ctx.Record() is already running elsewhere. For an example see above, with app.Use(iris.Compression)

[kataras]

Yes @Dexus, I see, the problem was that ctx.Record had a type check which allowed only *responseWriter to be wrapped (in order to not wrap twice the recorder) and also in the error handler there was no check for the response recorder's underline iris writer (e.g. compress response writer). This is fixed now:

iris/_examples/routing/http-errors/reset-body/main.go

Lines 1 to 42 in 39a1f00

	package main
	import (
	"fmt"
	"github.com/kataras/iris/v12"
	)
	func main() {
	app := newApp()
	app.Listen(":8080")
	}
	func newApp() *iris.Application {
	app := iris.New()
	app.Use(iris.Compression)
	app.OnAnyErrorCode(onErrorCode)
	app.Get("/", handler)
	app.Configure(iris.WithResetOnFireErrorCode)
	return app
	}
	// This is the default error handler Iris uses for any error codes.
	func onErrorCode(ctx iris.Context) {
	if err := ctx.GetErr(); err != nil {
	ctx.WriteString(err.Error())
	} else {
	ctx.WriteString(iris.StatusText(ctx.GetStatusCode()))
	}
	}
	func handler(ctx iris.Context) {
	ctx.Record()
	ctx.WriteString("This should NOT be written")
	// [....something bad happened after we "write"]
	err := fmt.Errorf("custom error")
	ctx.StopWithError(iris.StatusBadRequest, err)
	}

Is that the solution you were looking for?

[Dexus]

@kataras
after your change in 39a1f00 i don't get any response now together with app.Use(iris.Compression) and ctx.Record() and iris.WithResetOnFireErrorCode. I only get a blank page. If i remove the app.Use(iris.Compression) everything works fine again.

[kataras]

@Dexus that's strange, why you get an empty page? The error message doesn't shown up? Can you please create a repo that I can reproduce it? Thanks!!

[Dexus]

I will try to create a repo that have the exact same problem but without our code, this take some time, but try to finish it today.

[kataras]

I will try to create a repo that have the exact same problem but without our code, this take some time, but try to finish it today.

You can just create a simple jet view, with a runtime error, put the code you use, and the middlewares, I dont need your whole code base, just a small example that I can run and run until I fix the issue. Thanks @Dexus !

[Dexus]

@kataras here your example: https://github.com/Dexus/iris-bug-examples

please loog to the main.go and comment app.Use(iris.Compression) or uncomment it, to check what is wrong...

[kataras]

Oh is this the simplest example you could give me? :P I see many things like translation and view functions but it doesn't work it panics because locale files are missing, I will try to clean up, and get back to you soon. Also I want to know why you select to not use the Iris' i18n for localization which you can use on context methods and views as well, if there is something we can do to improve the iris i18n experience please let me know

[Dexus]

Just copy paste the most of it 😅 to not remove something that may have an impact. Von meinem iPhone gesendet

…

Am 26.07.2020 um 18:17 schrieb Gerasimos (Makis) Maropoulos ***@***.***>:  Oh is this the simplest example you could give me? :P I see many things like translation and view functions, I will take a look of it, and get back to you soon. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[kataras]

OK @Dexus give it a try

[Dexus]

It works now. Thanks @kataras

[kataras]

I thank you @Dexus, will take a look at the assesment for the certification of yours when I finish the documentation, I didnt forget it. I am freezing new features for v12.2.0.