roadrunner-server / roadrunner

🤯 High-performance PHP application server, process manager written in Go and powered with plugins
https://docs.roadrunner.dev
MIT License
7.92k stars 411 forks source link

[Feature Request] Worker fatal error handling #80

Closed grachevko closed 4 years ago

grachevko commented 5 years ago

For now if worker crashed with fatal error the stack trace are print to browser.

I have several suggestions:

Alex-Bond commented 5 years ago

@grachevko im working on Sentry integration. Maybe will release after the New Year.

wolfy-j commented 5 years ago

We are handing fatal errors on php end, same as with php-fpm, don’t symfony wrap them with custom error handler?

grachevko commented 5 years ago

@wolfy-j fatal error i mean is when php exit with non zero status code. In some cases it can send to sentry before exit, but it can't render user friendly error after.

While testing rr i was catch error where Sentry reach maximum nested function calls, for example. In this case rr have no option to report developer about error, only end user can.

wolfy-j commented 5 years ago

Ok, EventWorkerError (https://github.com/spiral/roadrunner/blob/master/cmd/util/debug.go#L19) contains all the info including error code.

Alex-Bond commented 5 years ago

I finished beta version of sentry integration - https://github.com/Alex-Bond/roadrunner/tree/sentry @wolfy-j any ideas how to "package" service? We probably have to write some sort of composer for RR that gonna add requirements and code to different files.

grachevko commented 5 years ago

In my opinion features like this must be out of the box because it wide uses, rather than needed for small count of projects.

Alex-Bond commented 5 years ago

@wolfy-j what do you think? We can include it in the main package. It just will not start if no DSN provided...

wolfy-j commented 5 years ago

Hi,

this is example of how we pack services: https://github.com/spiral/jobs/tree/2.0 https://github.com/spiral/php-grpc

If packed properly the only thing you have to do to connect it is to update your main.go file https://github.com/spiral/roadrunner/wiki/Building-Server https://github.com/spiral/roadrunner/wiki/Writing-Services

@grachevko @Alex-Bond since we can build rr in any form i can create separate repository or build with many other services included (i have been planning to do it anyway to have support for GRPC and Async Jobs out of the box anyway later this year).

But i will need tests for the sentry package :)

Alex-Bond commented 5 years ago

But i will need tests for the sentry package :)

Damn! I have writing tests :(

Alex-Bond commented 5 years ago

I wrote some tests. I don't know how to test listeners. @wolfy-j any suggestions?

P.S. @wolfy-j can you create branch sentry in this repo? I will create a pull request to it.

wolfy-j commented 5 years ago

Please check: https://github.com/spiral/roadrunner/wiki/Quick-Builds

If you'll pack your service similar to other services you will be able to build rr with sentry support by simply using (assuming sentry is in Alex-Bond/sentry):

{
  "packages": [
    "github.com/spiral/roadrunner/service/env",
    "github.com/spiral/roadrunner/service/http",
    "github.com/spiral/roadrunner/service/rpc",
    "github.com/spiral/roadrunner/service/static",
    "github.com/Alex-Bond/sentry"
  ],
  "commands": [
    "github.com/spiral/roadrunner/cmd/rr/http",
  ],
  "register": [
    "rr.Container.Register(env.ID, &env.Service{})",
    "rr.Container.Register(rpc.ID, &rpc.Service{})",
    "rr.Container.Register(http.ID, &http.Service{})",
    "rr.Container.Register(static.ID, &static.Service{})",
    "rr.Container.Register(sentry.ID, &sentry.Service{})"
  ]
}
$ vendor/bin/rr-build v1.0.0-local .build.json
Alex-Bond commented 5 years ago

@wolfy-j what about event listeners?

wolfy-j commented 5 years ago

@Alex-Bond you can do deep integration test like here https://github.com/Alex-Bond/roadrunner/blob/sentry/service/http/service_test.go#L347

Alex-Bond commented 5 years ago

@wolfy-j no. i mean how do you integrate event listeners? https://github.com/Alex-Bond/roadrunner/blob/sentry/cmd/rr/main.go#L50

wolfy-j commented 5 years ago

Ah. Request http dependency here https://github.com/Alex-Bond/roadrunner/blob/sentry/service/sentry/service.go#L18

Like here: https://github.com/Alex-Bond/roadrunner/blob/sentry/service/static/service.go#L23

wolfy-j commented 5 years ago

I would also propose you to write service code only without full fork of RR. It would be much easier to distribute this way.

Alex-Bond commented 5 years ago

Here what come with: https://github.com/UPDG/roadrunner-sentry

wolfy-j commented 5 years ago

Can you please make version release? I’m think to add it into ce build.

Alex-Bond commented 5 years ago

@wolfy-j done

rustatian commented 4 years ago

@Alex-Bond @wolfy-j @grachevko I am going to close this issue due to age. Feel free to reopen it if you have any questions.