Need to support panic recovery in the request handlers

[ysaakpr]

ocpp-go/ocpp1.6/central_system.go

Lines 502 to 509 in c34f6ad

	go func() {
	switch action {
	case core.BootNotificationFeatureName:
	confirmation, err = cs.coreHandler.OnBootNotification(chargePoint.ID(), request.(*core.BootNotificationRequest))
	case core.AuthorizeFeatureName:
	confirmation, err = cs.coreHandler.OnAuthorize(chargePoint.ID(), request.(*core.AuthorizeRequest))
	case core.DataTransferFeatureName:
	confirmation, err = cs.coreHandler.OnDataTransfer(chargePoint.ID(), request.(*core.DataTransferRequest))

What happen if a panic happens in a request handler in this go routine?

My server keep re starting since its a go routine, and my request level panic handler not working here. Need a way to handle the panic situations. Or have proper panic recovery for all the internal go routines.

defer func() {
			r := recover()
			if r != nil {
				var err error
				switch t := r.(type) {
				case string:
					err = errors.New(t)
				case error:
					err = t
				default:
					err = errors.New("Unknown error")
				}
				// sendMeMail(err)
				fmt.Println("recivered panic:", err)
				cs.sendResponse(chargePoint.ID(), confirmation, err, requestId)
			}
		}()

This is what is did in the start of the go routine to fix it.

[lorenzodonini]

The callback expects either a response or an error. Currently a panic within the same goroutine will break the request - response flow for that connection (which can only be fixed with a re-connection).

Why does the handler panic anyway?

[ysaakpr]

the panic will cause the system to exit if not recovered.

Why panic, may be a bad coder/coding. and some libraries cause panic instead of error in its error cases, and unless handled carefully all these can cause the entier server to go down. Which is not a good behaviour

Isn't it better to recover the panic, or give the flexibility to handle a panic at root level, by setting a panic handler.

[lorenzodonini]

My point was simply about "bad coders" being responsible for their own implementation. If a handler is panicking, it should be fixed in there.

That being said your suggestion is definitely a good improvement, I'll add it in a PR.

[ysaakpr]

My point was simply about "bad coders" being responsible for their own implementation. If a handler is panicking, it should be fixed in there.

Yes... it should be fixed in there, in our case there are many handlers performing ocpp message handling, and there is no common place in the library where i can wrap the logic of panic recovery. From a library perspective we need some handler injection possible at the root level,

That being said your suggestion is definitely a good improvement, I'll add it in a PR.

Thanks for taking it, better to accept it as a panic handler in the configurations of Server