javascript-sdk icon indicating copy to clipboard operation
javascript-sdk copied to clipboard

please change your code to throw in error conditions rather than return null

Open olsonpm opened this issue 2 years ago • 5 comments

How would the enhancement work?

So far I've seen this pattern in createInstance and createUserExperience

  • createInstance should throw instead of return null when an error occurs. i.e. the try/catch should be removed here
  • createUserExperience should throw when input is invalid with an error message explaining why. Returning null gives the devs no clue as to what went wrong or why - just that something went wrong. It is an error and should be treated as such

When would the enhancement be useful?

because returning null in error scenarios doesn't make sense. Devs naturally expect an error to bubble with the proper stack trace and error information rather than the function to return successfully and have to write the error edge case themselves.

olsonpm avatar Jan 30 '23 20:01 olsonpm

Thanks for raising this issue @olsonpm - nice catch, I think I agree with you on the approach of throwing an exception instead of returning null for these two cases. Second case with createUserExperience might benefit from an additional error log as well.

Will chat with the team internally to verify if there's any contextual significance before proceeding further.

opti-jnguyen avatar Feb 02 '23 23:02 opti-jnguyen

@olsonpm - due to this being a breaking change for current users of the SDK, we'll be putting further development of this on the backburner to be done at another date, likely as part of a greater refactor to the JavaScript SDK. Will keep this issue open but unresolved until then.

opti-jnguyen avatar Mar 08 '23 20:03 opti-jnguyen

Makes sense. I really appreciate the update

olsonpm avatar Mar 08 '23 20:03 olsonpm

@olsonpm - due to this being a breaking change for current users of the SDK, we'll be putting further development of this on the backburner to be done at another date, likely as part of a greater refactor to the JavaScript SDK. Will keep this issue open but unresolved until then.

The change to having createInstance return Client | null instead of just Client in 4.9.2 was already a breaking change that was released as a patch update.

philBrown avatar Mar 29 '23 22:03 philBrown

Internal ticket created FSSDK-9572

Tamara-Barum avatar Aug 03 '23 16:08 Tamara-Barum

v6 of optimizely-sdk has been released with these changes. @olsonpm

raju-opti avatar May 29 '25 19:05 raju-opti