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

Change return type of _encode functions in bit_array module #750

Open
richard-viney opened this issue Nov 27, 2024 · 1 comment · May be fixed by #751
Open

Change return type of _encode functions in bit_array module #750

richard-viney opened this issue Nov 27, 2024 · 1 comment · May be fixed by #751
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@richard-viney
Copy link
Contributor

richard-viney commented Nov 27, 2024

I'd like to propose a breaking change in the bit_array module:

  • Change the return type of the base64_encode, base64_url_encode, and base16_encode functions from String to Result(String, Nil).
  • They then return Error(Nil) if passed an unaligned bit array.

This came up as part of my work on unaligned bit arrays on JS. Currently on Erlang an exception is thrown, which I'm not sure is explicitly intended behaviour and is a bit awkward to deal with.

Alternatives to changing the return type:

  • Don't change the existing behaviour on Erlang, and throw an exception from the JS implementation on unaligned bit arrays.
  • Silently pad unaligned bit arrays with zero bits to reach a whole number of bytes, then encode that. This is probably a bit too magical/implicit though.

Thoughts?

@lpil
Copy link
Member

lpil commented Nov 27, 2024

Oh dear, what a poor oversight from me.

We definitely do not want to crash. Moving to using result would require making new functions with new names, deprecating the old ones, making new functions with the old names, and then deprecating the intermediate functions. Given the difficulty of this process I think it would be sensible to pad with zeros instead.

There is an outstanding ticket to do the same for bytes tree #320

@lpil lpil added help wanted Extra attention is needed bug Something isn't working labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants