panjf2000 / ants

🐜🐜🐜 ants is the most powerful and reliable pooling solution for Go.
https://ants.andypan.me/
MIT License
12.69k stars 1.36k forks source link

[Feature]: Add panicHandlerBuilder #310

Closed PeterlitsZo closed 9 months ago

PeterlitsZo commented 9 months ago

Description of new feature

Scenarios for new feature

In my opinion, panicHandlerBuilder will be much helpful. Here is my view:

So, I think if we can have panicHandlerBuilder.

Breaking changes or not?

Yes

Code snippets (optional)

panicHandlerBuilder := (moreInfomation any) func(any) {
    mi := moreInfomation.(*MoreInfomation)
    logger := mi.Logger
    ctx := mi.ctx

    return func(e any) {
        logger.Error(ctx, map[string]any{ "err", e })
    }
}

pool, _ := ants.NewPool(poolSize, ants.WithPanicHandlerBuilder(panicHandlerBuilder))

pool.SubmitWithMoreInfomation(task, &MoreInfomation{
    ctx: ctx,
    logger: logger,
})

(Are Go generics useful? But I notice that the go version of the project is not >= 1.18)

Alternatives for new feature

pool.Submit(function() {
    defer function() {
        if err := recover(); err != nil {
            handlePanic(ctx, logger, err)
        }
    }

    task()
})

I do not think it looks pretty - If we have panicHandler, why not have panicHandlerBuilder?

Additional context (optional)

I think I am able to send a PR (maybe...).

PeterlitsZo commented 9 months ago

hello?

panjf2000 commented 9 months ago

I appreciate your work here, but your feature request seems like a lot of code changes, and I don't see why a good deal of code changes are required to be added to ants when a simple wrapper outside ants will also work, something like:

type PoolWrapper struct {
    pool *ants.Pool
}

func (p *PoolWrapper) Submit(task func(), info *MoreInfomation) error {
    p.pool.Submit(func() {
        defer func() {
            if err := recover(); err != nil {
                handlePanic(info.ctx, info.logger, err)
            }
        }()

        task() 
    })
}

func handlePanic(ctx context.Context, logger Logger, err any) {
    logger.Error(ctx, map[string]any{"err", err})
}
PeterlitsZo commented 9 months ago

Yes, you are right. Thanks for your great work.