cookie
cookie copied to clipboard
Feature: Throw TypeErrors in `serialize`/`parse` with .code
As requested in #162 as a non-breaking change prior to a change of error messages for a future major release, standard Node error codes have been added to each of the parse
and serialize
functions' TypeError throws.
This should allow for a period of migrating for library users to error-handling via the .code
error property, as opposed to the sole .message
property, before they change (potentially breaking).
This PR
- Amends existing tests for
parse
andserialize
to test for the correct error message and error code.- The above requires a shared helper function extracted to a separate file
compare-error.js
- The above requires a shared helper function extracted to a separate file
- Adds a
serialize
sameSite test for non-string/true argument values. - Adds
.code
properties with standard Node error codes to each error throw in both theparse
andserialize
functions.
Looks like asserting throws with a POJO was breaking backwards compatibility with the action runs. This change should hopefully be backwards compatible with the versions now.
Could I get some guidance on exactly what needs fixing here for the action runs? I'm a little confused as to why some tests that passed last run now suddenly fail after only changing the indentation for one of the files (which understandably was causing some of the later Node version tests to fail) As far as I'm concerned, the original issue regarding the test object/callback has been resolved, as has the indentation and on my end, I remember Node v0.10 and v0.12 passing respectively.
Could I get some guidance on exactly what needs fixing here for the action runs?
Ah, sorry yeah I should have been more clear. I don't think you need to fix this in this PR. I think we have been considering just dropping the coveralls stuff, but it is unclear what we want bigger picture. I don't want to block landing things on this kind of unrelated stuff, so what I was thinking is I would run these tests locally once to check, but I dont see anything which should break so I hope we could just skip the check and force re-run each test run since it got past that part.