Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel Octane Compatibility #121

Open
ldiebold opened this issue Aug 30, 2021 · 13 comments
Open

Laravel Octane Compatibility #121

ldiebold opened this issue Aug 30, 2021 · 13 comments
Labels
bug Something isn't working

Comments

@ldiebold
Copy link

When making a request that's validated with Orion, using Octane, the second request skips validation.
This is likely due to the nuances in making a package octane ready.

I did some digging into the source code, but started to hit the limits of my knowledge. Reproduced the bug with a basic app instead:

https://github.com/ldiebold/orion-octane-request-test

Note that I'm using sail to make it easier to setup octane :)

here's some images of the error:

First request:
orion-first-request

Second request:
orion-second-request

Also worth noting that a composer dump will allow me to validate the initial request again (probably because dumping forces octane to copy the app again)

Here's Muhamed Said's video on Octane if anyone wants a deeper understanding on this!

Thanks @alexzarbn :)

@alexzarbn
Copy link
Member

Hi @ldiebold,

Thank you for raising this issue! I haven't checked Orion's compatibility with Octane, but it looks like there might be a few caveats. Will try to include it as part of the next release 👌🏻

@alexzarbn alexzarbn changed the title Request validation skipped after first success when using octane Laravel Octane Compatibility Sep 1, 2021
@alexzarbn alexzarbn added the bug Something isn't working label Sep 1, 2021
@AbdullahFaqeir
Copy link

I've already done an octane compatibility fix, I can handle this, if no one is working on it.

@ldiebold
Copy link
Author

I've already done an octane compatibility fix, I can handle this, if no one is working on it.

@AbdullahFaqeir amazing! Is there a fork I can use? 🙂

@nathan-io
Copy link

nathan-io commented Dec 22, 2021

Hi @ldiebold wondering if you've had a chance to investigate this any further? I'll be serving an API with Octane and was hoping to build it with Orion!

I'm wondering if it could be as simple updating the flush config in config/octane.php:

    /*
    |--------------------------------------------------------------------------
    | Warm / Flush Bindings
    |--------------------------------------------------------------------------
    |
    | The bindings listed below will either be pre-warmed when a worker boots
    | or they will be flushed before every new request. Flushing a binding
    | will force the container to resolve that binding again when asked.
    |
    */

    'warm' => [
        ...Octane::defaultServicesToWarm(),
    ],

    'flush' => [
    ],

@ldiebold
Copy link
Author

@nathan-io not sure... But let me know if that works!

Would also love to move to octane. Even in local dev, I can really feel the difference.

We just need to be sure it works, because the issue I had is that it's skipping Request Validation. If validation is skipped, that's a pretty serious security concern.

@nathan-io
Copy link

@ldiebold I'm still building out the models, policies, etc., will probably be a bit before I can start configuring and testing Orion.

I've made some notes about Octane, here are some resources I've found which may be helpful in diagnosing the issue:

  • "Dependency Injection & Octane" from Laravel Docs
  • "Coding With Laravel Octane in Mind" here
  • "Things to Consider" section here

@ldiebold
Copy link
Author

ldiebold commented Apr 5, 2022

@alexzarbn are you open to pull requests for this?
We LOVE orion at work, and could probably dedicate some time to supporting octane!

As an aside... I started using another backend framework recently and can't begin to tell you how much I miss Orion/Laravel! It's an absolute joy to use ❤️.

@alexzarbn
Copy link
Member

@ldiebold Sure, any PRs are welcome, and it would be great to have support for Octane!

Oh, what framework are you using now? Why moved away from Laravel?

@ldiebold
Copy link
Author

ldiebold commented Apr 5, 2022

Great, will look into it today!

Still using Laravel at current job, though new job is using a node backend.

Laravel spoils us! It has an answer, and a "way" for every backend problem I've ever had.

And some of these other frameworks don't even have documentation for some of the core functionality. It's crazy!

... Anywho, getting off topic 😅

@michaelrimbach
Copy link

Is this package working with Laravel Octane already?

@binaryfire
Copy link

binaryfire commented Jun 12, 2023

I'd love to use Orion for my Octane app. Has anyone solved this problem?

When using Octane, each request doesn't have a completely fresh start like FPM applications. Instead, Octane keeps the application state in memory between requests. @ldiebold's error is definitely being caused by something stateful being persisted across requests.

I generally have to avoid static classes and singletons in my Octane app to avoid memory leaks. No obvious problems jump out at me when I do a quick search of Orion's codebase, but I'm not an expert.

EDIT: Never mind, I misread how Post::class was being used. I could have sworn I saw protected static $model = Post::class; !

I'll spend some time trying to debug this today. Hopefully I can figure it out

@binaryfire
Copy link

This is working for me in Laravel 10 with Octane (Swoole). Maybe something has changed in Octane since this issue was opened.

@ldiebold Were you using Roadrunner or Swoole? Here's my Laravel 10 test project: https://github.com/binaryfire/laravel-orion-octane-test . Could you spin it up and see if it works for you?

@michaelrimbach
Copy link

It's fully working with Octane and Roadrunner for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants