Handle function call errors more gracefully in routes #10653

New Issue

GiteaMirror · 2025-11-02T09:14:25-06:00

GiteaMirror commented

2025-11-02 09:14:25 -06:00

Originally created by @wxiaoguang on GitHub (Apr 14, 2023).

Feature Description

Example: help more problems like #24118

Before

	count, err := actions_model.CountRunners(ctx, opts)
	if err != nil {
		ctx.ServerError("AdminRunners", err)
		return
	}

There are some problems:

The function names changes during refactoring, so the ServerError frequently gets out-of-sync.
People have to copy&paste duplicate code again and again.
The return could be forgotten, a lot of times.

Proposal 1

Use generic, let a function like ErrorProof handles server errors automatically.

ErrorProof can use reflect to get the target function's name, it doesn't affect performance because in most cases there is no "internal server error".

	count, err := ctx.ErrorProof(actions_model.CountRunners, ctx, opts))
	if err != nil {
		return
	}

Proposal 2

Return the err directly, make our handler framework support such return value for handler functions (with stacktrace):

	count, err := actions_model.CountRunners(ctx, opts)
	if err != nil {
		return ctx.ErrorWithStack(err)
	}

Feasible?

These are just some early ideas, haven't tried whether it is really feasible, but if people like this proposal, I will spend some time to try.

I really prefer the second one, it looks clear and intuitive

Originally created by @wxiaoguang on GitHub (Apr 14, 2023). ### Feature Description Example: help more problems like #24118 ### Before ```go count, err := actions_model.CountRunners(ctx, opts) if err != nil { ctx.ServerError("AdminRunners", err) return } ``` There are some problems: 1. The function names changes during refactoring, so the ServerError frequently gets out-of-sync. 2. People have to copy&paste duplicate code again and again. 3. The `return` could be forgotten, a lot of times. ### Proposal 1 <details> Use generic, let a function like `ErrorProof` handles server errors automatically. `ErrorProof` can use reflect to get the target function's name, it doesn't affect performance because in most cases there is no "internal server error". ```go count, err := ctx.ErrorProof(actions_model.CountRunners, ctx, opts)) if err != nil { return } ``` </details> ### Proposal 2 Return the err directly, make our handler framework support such return value for handler functions (with stacktrace): ```go count, err := actions_model.CountRunners(ctx, opts) if err != nil { return ctx.ErrorWithStack(err) } ``` ### Feasible? These are just some early ideas, haven't tried whether it is really feasible, but if people like this proposal, I will spend some time to try. I really prefer the second one, it looks clear and intuitive

GiteaMirror added the type/proposal type/feature labels 2025-11-02 09:14:25 -06:00

GiteaMirror closed this issue

2025-11-02 09:14:27 -06:00

GiteaMirror commented

2025-11-02 09:14:32 -06:00

@silverwind commented on GitHub (Apr 21, 2023):

In a sane language with exceptions, I would say throw those errors, but given golang has no exceptions besides panic/recover (I'm not sure it should those can or should be used as a rudimentary exception system), error return is probably the next best thing.

Ideally there should be linting for this but I fear it might require us to implement this linting ourselves, or maybe some existing linters can already help.

@silverwind commented on GitHub (Apr 21, 2023): In a sane language with exceptions, I would say `throw` those errors, but given golang has no exceptions besides `panic`/`recover` (I'm not sure it should those can or should be used as a rudimentary exception system), error return is probably the next best thing. Ideally there should be linting for this but I fear it might require us to implement this linting ourselves, or maybe some existing linters can already help.

GiteaMirror commented

2025-11-02 09:14:32 -06:00

@lunny commented on GitHub (May 29, 2023):

I like proposal 2, I have ever implemented a web framework https://gitea.com/lunny/tango support to receive one or two return error parameters like

func a() error
or
func b() (statusCode int, error)

@lunny commented on GitHub (May 29, 2023): I like proposal 2, I have ever implemented a web framework https://gitea.com/lunny/tango support to receive one or two return error parameters like `func a() error` or `func b() (statusCode int, error)`

GiteaMirror commented

2025-11-02 09:14:32 -06:00

@wxiaoguang commented on GitHub (May 29, 2023):

I also have done similar thing in my Java/PHP/Golang web frameworks.

Usually I make the return type as an interface, then the handler could have the full control for what it wants to output.

The common responses could be provided by builtin response classes:

func handler(ctx *web.Context) Response {
    return ctx.JSON()
}

func handler(ctx *web.Context) Response {
    return ctx.HTML()
}

func handler(ctx *web.Context) Response {
    return ctx.PlainText()
}

func handler(ctx *web.Context) Response {
    return ctx.ServerError()
}

func handler(ctx *web.Context) Response {
    return &MyResponseWriter{  write status, write headers, write body ... }
}

@wxiaoguang commented on GitHub (May 29, 2023): I also have done similar thing in my Java/PHP/Golang web frameworks. Usually I make the return type as an interface, then the handler could have the full control for what it wants to output. The common responses could be provided by builtin response classes: ```go func handler(ctx *web.Context) Response { return ctx.JSON() } func handler(ctx *web.Context) Response { return ctx.HTML() } func handler(ctx *web.Context) Response { return ctx.PlainText() } func handler(ctx *web.Context) Response { return ctx.ServerError() } func handler(ctx *web.Context) Response { return &MyResponseWriter{ write status, write headers, write body ... } } ```

GiteaMirror commented

2025-11-02 09:14:33 -06:00

@wolfogre commented on GitHub (Jul 5, 2023):

I agree, it surprised me the first time I saw this:

	v := DoSomething(ctx)
	if ctx.Written() {
		return
	}
	// then the v is valid

@wolfogre commented on GitHub (Jul 5, 2023): I agree, it surprised me the first time I saw this: ```go v := DoSomething(ctx) if ctx.Written() { return } // then the v is valid ```

GiteaMirror commented

2025-11-02 09:14:33 -06:00

@wxiaoguang commented on GitHub (Jul 22, 2023):

No time for it. Close.

@wxiaoguang commented on GitHub (Jul 22, 2023): No time for it. Close.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#10653