fix: Remove validation error handler option error from various methods of Parse.Object

[dplewis]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

You can pass an option in set to handle validation errors. The problem is this option isn't well documented and isn't widely used. This method of error handling is called backbone and was removed in 2.0.0. There are two errors (invalid key name and invalid acl) that are being checked. This could potentially be a breaking change if a user is setting invalid fields and never saving as saving would throw the errors.

object.set('$$$', 'o_O', {
  error: function (thisObj, err) {
    // err is ParseError.INVALID_KEY_NAME but doesn't have an error message.
  },
});

Closes: #2104

Approach

• Remove backbone options.error
• Add error message for Parse.Error.INVALID_KEY_NAME
• Improve documentation for chaining
• set now throws validation errors

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)

[parse-github-assistant]

Thanks for opening this pull request!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (fa34827) to head (e4e3929).
Report is 18 commits behind head on alpha.

Additional details and impacted files

@@            Coverage Diff            @@
##             alpha     #2445   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         6256      6256           
  Branches      1452      1446    -6     
=========================================
  Hits          6256      6256           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mtrezza]

Now is a good time to merge any breaking changes. We can release with Parse Server 8.

[mtrezza]

BREAKING CHANGE: Removes the validation error option from Parse.Object.set, Parse.Object.setACL and instead throws on validation errors. For example, instead of using Parse.Object.set(key, value, { error: ... }) wrap Parse.Object.set(key, value) into a try/catch block.

@dplewis Is this proposed changelog entry correct?

[dplewis]

There is also Parse.Object.unset and Parse.Role.setName that throw errors now.

Edit: Basically setting / updating a field with invalid key or acl will throw error now. Before you would have to save first to get the error. If a developer fixed all errors before updating to using this change there shouldn't be any issues. If they just set fields and never saved then that would be a breaking change

[mtrezza]

I guess these are 2 changes then:

1. remove error option
2. throw not at save but at set et al.

Could we do only (1) and not (2), if it's not necessary for (1)? In any case, if the primary purpose of this PR is (1), then this would be a breaking change either way, because it's possible that someone is using it, right?

[dplewis]

remove error option

If you didn't pass an error option the error would be slienced but now it's thrown

throw not at save but at set et al.

The error would get thrown before you get a chance to save.

[mtrezza]

So the original behavior is:

• if error option is set, then error is thrown at .set
• if error option is not set, then error is thrown at .save

New behavior with this PR:

• error is thrown at .set (error option doesn't exist anymore)

Is that correct?

[dplewis]

This is correct. Users have reported that save didn’t throw at all like #1386 and there were a few other examples but I can't find them.

[mtrezza]

Do you think it makes more sense to throw on .set or on .save? Throwing on .save seems to make more sense to me, because .set seems more like a non-op. Do we have any other similar examples in the SDK?

[dplewis]

I think it should throw on .set because it's easier to debug where the error is in the stack trace. Also this PR adds a message for INVALID_KEY_NAME error so you can get the name of the key. The documentation and return types are improved and consistent with the rest of the SDK like query chaining. .set returning Parse.Object | boolean is weird.

[mtrezza]

BREAKING CHANGE: Removes the error handler option error from Parse.Object.set, Parse.Object.setACL, Parse.Object.unset, Parse.Role.setName and instead throws on validation error. Previously, if the error option was set, the handler was invoked if a validation error occurred on Parse.Object.set, and if no handler was set, an error was thrown on Parse.Object.save. The new behavior is that an error is thrown at Parse.Object.set. For example, instead of using Parse.Object.set(key, value, { error: ... }) wrap Parse.Object.set(key, value) into a try/catch block.

@dplewis Is this proposed changelog entry correct?

[dplewis]

LGTM!

[parseplatformorg]

🎉 This change has been released in version 6.0.0-alpha.1

[parseplatformorg]

🎉 This change has been released in version 6.0.0