Panic based framework QIWI

As a backend developer i have recently been immersed in NestJS at my company. One of the features that captivated me was the ability to throw exceptions like throw new AppBadRequest() to handle HTTP messages. Being able to break the whole http request and returning the http message mid code is beautiful
However, when coding in golang again i have been missing this thing, i dont know if already exists such a framework but i have build my own, following this sample project: GitHub - dalton02/Qiwi-Base-Backend: Estrutura base para serviços backend utilizando framework Qiwi e Prisma

It actually has a university project of a blog that i am working on, the framework has a json validator using the reflector, and has some cool stuff, like custom protected and public routes for jwt.
Later i am thinking of expeding this to being more customizable, and it would be cool if you guys give me some tips, the whole framework is in the “requester” folder.

Example Implementation

func InsertPost(db *sql.DB, postagem postagensDto.NovaPostagem, response http.ResponseWriter) int {
    var lastInsertID int
    err := db.QueryRow("INSERT INTO postagem (tipo, titulo, conteudo, usuario_id, tags) VALUES ($1, $2, $3, $4, $5) RETURNING id",
        postagem.Tipo, postagem.Titulo, postagem.Conteudo, postagem.UsuarioId, pq.Array(postagem.Tags)).Scan(&lastInsertID)
    
    if err != nil {
        httpkit.AppConflict("A conflict has occurred: "+err.Error(), response)
        // If this function is called in high order functions, the panic will go all the way back to the original http.handleFunc
    }
    return int(lastInsertID)
}` 

Comparison with NestJS

import { Injectable, ConflictException } from '@nestjs/common';

@Injectable()
export class PostService {
    async insertPost(postagem: NovaPostagem): Promise<number> {
        try {
            const result = await this.postRepository.insert(postagem);
            return result.identifiers[0].id; // Returning the ID of the last post
        } catch (error) {
            throw new ConflictException('A conflict has occurred: ' + error.message);
        }
    }
}`

Obs: Sorry if some things are in portuguese in the codebase

There was some discussion about this recently so I’m not going to re-hash it, but in general, don’t panic. Check out the responses on this thread for more info:

As he said, and as has been discussed recently, Golang’s error handling is fascinating, and you’ll love it if you get used to this style of code.

I mean, I really love the way Go handles errors. I have been using some of the Go syntax in TypeScript as well, and I know you guys hate using panic, but I have tested every edge case and made some 10,000 requests per second to see if it would impact performance. I just think this approach I take specifically for sending HTTP messages is kind of good. I only use this approach to send early messages to the client. If you see the struct I made, every panic is being recovered and logged. I don’t even mean it’s better than the original way; I’m just showing a cool alternative that makes the code clean and kind of short in some ways.

It does not make anything clean at all. So you just panic and recover somewhere else inside your library? Because all I see is a lot of confusing exported functions (not even methods) which instead of simply return an error panic. This doesn’t seem good, it feels wrong, just wrong. When someone says he tested edge cases that means there are still 100500 ways of breaking everything.

This one actually looks like recreation of try/catch in golang which goes straight against one of the core things, errors as values. If I want to throw an exception I code in python. After all this years with go, I want to write err != nil and get this err as the last return value from a function.

Obviously, we can’t stop a developer from doing stupid things. As I observed, golang’s design is so simple and charming that it makes some people think strange things.
When they think best practices are wrong, they don’t think their thinking is wrong unless they encounter bad consequences.
panic is only caught in defer and executes some logic, which causes leaks when writing code that panic but isn’t caught.
It’s much better to let the caller know that an err will occur, rather than panic implicitly.

1 Like

I think “stupid” is rude and too strong of a word here. I am onboard with it not being idiomatic, and I like how Go treats errors as values. But there are a lot of frameworks/languages/language designers out there who prefer other paradigms. I don’t think it’s great to shoehorn those into Go per se (like how we treat errors as values is one of the things that makes the language different!) but try not to be rude to new community members.

I mean sorry if you fell like this is stupity? This works on my companys project that i am developing and was very idiomatic to me, i just wanted to share to see if someone like this approach dude, chill…

I’m sorry, my words are very blunt, maybe the translator is a problem. What I’m trying to say is that doing something too easy (like? I can’t think of a better word).

I’ve had bad times where debug is weird because of panic everywhere and leaks if defer misses something. With good error passing, both the caller and the provider work well together.
Also, callers don’t like the fact that every function call has a hidden panic. But if you don’t explicitly state what’s going to happen, then you have a big hurdle to debug, not to mention the risk of possible leaks.
http.handler’s recover is designed as an insurance policy in case of an unexpected panic that prevents the principal service from being interrupted. When such insurance measures are triggered voluntarily, it is a very unusual thing. If you just want the call stack, you should call the relevant function instead of panic.
Finally, I apologize if I offended you with what I said earlier.
But I also think it’s not a good way to learn if you don’t subscribe to best practices. Or should be “eat a sting wisdom” will pay attention to some problems.

There are people who are not natives, and sometimes the language might seem rude, when there were no bad intentions behind it.

But besides this, I don’t think that error handling has become a paradigm of some sort. It’s part of a language design and might have best practices. And it goes not just for Golang. At the same time, when ppl write on this forum asking for feedback, they should be ready for feedback. I’d like to see clear reasoning why they did this and why it’s better than anything else. Or I’ll argue from the point of my experience and knowledge.

Do you work on this project alone, or is it your team’s solution? Was this design discussed? Can you give any good reason for why it should be used instead of errors handling? Anything besides economy of code strings.

It kinda is, but to make the golang way i can just return the http message in the controller, and keep the panic recover to only return a “internal server error” as usual, the only thing that is kinda annoying is that in my service i may have to return multiple error variables to send differents http messages, like in a service that try to make a new account to you, i have to return differents errors to: “email already in the database”,“login already in the database”, and etc. I mean i would return something like:
data, errEmail, errLogin, errSomethingElse := accountservice.CreateAccount(login,email,password)

But i guess is the way, if somebody else is gonna use it, but other than that i have not seem a problem with the panic itself to cause leaking or something. Thanks for the feedback

Aren’t errEmail, errLogin, errSomethingElse error interfaces?
The general way to handle it would be like this:

data, err := accountservice.CreateAccount(login,email,password)
if err!=nil{
   if errors.Is(err,errEmail){
        // do errEmail ...
   }
   if errors.Is(err,errLogin){
        // do errLogin ...
   }
   if errors.Is(err,errSomethingElse){
        // do errSomethingElse ...
   }
}

This is just a simple example, and the corresponding handling of specific errors may be called at a higher level.

2 Likes

Wait, what? Why? error in go is an interface. It has Error() string method same as support for wrapping/unwrapping. Okay, if you want to send different codes it’s fine and pretty obvious. But you can split the logic of request processing in a way so functions which e.g. returns errors connected to bad request are in one place, those which has internal error to another. Or you can use errors package to get even more flexibility. E.g.,

package handler

import (
    "errors"
    "fmt"
    "net/http"
)

// Define as many custom errors as you want...
// It can be a simple string or even a more complex struct. 
var (
    ErrMailExists    = errors.New("email already exists")
    ErrLoginExists   = errors.New("login alrady exists")
    ErrSomethingElse = errors.New("something else")
)

func Handler(w http.ResponseWriter, r *http.Request) {
    if err := createUser(r); err != nil {
        code := http.StatusBadRequest

        // Depending on your logic it can be a switch,
        // one time check or whatever you prefer...
        if errors.Is(err, ErrSomethingElse) {
            code = http.StatusInternalServerError
        }

        http.Error(w, err.Error(), code)
        return
    }

    w.WriteHeader(http.StatusOK)
}

func createUser(r *http.Request) error {
    var login, email string
    // Exctract values from request...

    // Do whatever logic you need to do, with any error return you like.
    if inDb(login) {
        return fmt.Errorf("login %q: %w", login, ErrLoginExists)
    }

    if inDb(email) {
        return fmt.Errorf("email %q: %w", email, ErrMailExists)
    }

    if err := addToDb(login, email); err != nil {
        return fmt.Errorf("cannot add %s/%s into db: %w", login, email, err)
    }

    return nil
}

func inDb(s string) bool {
    // Check if exists...
    return false
}

func addToDb(login, email string) error {
    // You can use db connection here and wrap
    // error if there was one...
    return ErrSomethingElse
}

1 Like

I have in fact not looked at that, lol. I have been studying more complex stuff in go and forgot to look in to the basics. Thanks guys

Brainstorming for a more idiomatic method, for what it’s worth:

// Wrap all errors in a HTTPError that includes the status code
type HTTPError struct {
    err error
    status_code int
}

func BadRequest(err error) *HTTPError {
    return &HTTPError{err, http.StatusBadRequest}
}
func Conflict ...
func Forbidden ...
func NotFound ...
...

func SomeAPIEndpoint(w http.ResponseWriter, r *http.Request) *HTTPError {
    err := DBOperationWrapper()
    if err != nil {
        return err
    }
    ...
    if err != nil {
        return BadRequest(errors.New("Bad data received"))
    }
    return nil
}

func DBOperationWrapper() *HTTPError {
    ...
    if conflict_occurred {
        return Conflict(errors.New("A conflict has occurred"))
    }
    ...
    return nil
}