Skip to content

safety catch #7

Closed
Closed
@dbu

Description

@dbu

@dbu
could we have a magic "first" plugin that is inserted by the PluginClient that counts calls and prevents infinite restarts?

@joelwurtz
Not a fan of doing this as the boundaring between infinite restart and many redirection request can be thin sometimes.

@sagikazarmark
This could be a good feature I think, but it should not be automatically added. Can be good in a "fire&forget" situation, where you want to limit the number of restarts.

@dbu
maybe have an option to define how many restarts should be allowed?
we could also just accept a plugin for that, but thats very implicit and
not happening by default. and imho this is something that should happen
by default to protect people not aware of the risk. endless redirects
are pretty simple to create if the server is doing something stupid
(like redirect to the same url you are already on)

@sagikazarmark
An option sounds good to me, but it should be optional. I could imagine a setter for this option value, but the constructor should probably be limited to what we already have there.

@joelwurtz
Actually any plugin using the $first callable or doing retry must implement this mechanism (it's already done in the RetryPlugin and RedirectPlugin) we can maybe provide a SafePluginClient implementation (which extends the current one) to add this behavior ?

@dbu
i would see an $options argument to the PluginClient that allows to overwrite a default value (which could be 10 or 20 or something even larger). the thing is that the architecture is prone to endless loops. some people won't read documentation and end up with too many nested calls or even worse things when they run into this with a custom plugin. having something on by default that can be tweaked / disabled sounds better to me.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions