semian icon indicating copy to clipboard operation
semian copied to clipboard

Don't delegate to bulkhead, it might be nil

Open dalehamel opened this issue 7 years ago • 2 comments

What

Prevent a nil pointer dereference if attempting to read the name (or any other attribute delegated to bulkhead) of a protected resource without a bulkhead.

Why

Since bulkheads are now optional, we shouldn't delegate anything explicitly to them, as this can result in a nil pointer dereference.

How

The name is attempted to be read from the bulkhead first, falling back to the circuit breaker. If neither works, it will be nil, but it won't cause an exception to be raised from a nil pointer dereference.

Every other attribute has a safe accessor method defined, which will return a default value (same as for an unprotected resource) if there is no bulkhead.

dalehamel avatar Apr 03 '17 16:04 dalehamel

We might just want "allow_nil" here, not sure what the best behavior is.

dalehamel avatar Apr 03 '17 16:04 dalehamel

Yeah, I think we should consider just doing allow_nil. This is definitely an error, I don't think we should mock every call.

@sirupsen looks like allow_nil is a rails thing, if we are just using forwardable we might need to manually delegate things to make them safe when the delegate is nil.

dalehamel avatar Apr 04 '17 13:04 dalehamel