javascript-sdk
javascript-sdk copied to clipboard
please change your code to throw in error conditions rather than return null
How would the enhancement work?
So far I've seen this pattern in createInstance and createUserExperience
createInstanceshould throw instead of return null when an error occurs. i.e. the try/catch should be removed herecreateUserExperienceshould throw when input is invalid with an error message explaining why. Returningnullgives 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.
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.
@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.
Makes sense. I really appreciate the update
@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.
Internal ticket created FSSDK-9572
v6 of optimizely-sdk has been released with these changes. @olsonpm