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

fix: Parse.Query.subscribe() does not return a rejected promise on error in Cloud Code Triggers beforeConnect or beforeSubscribe #1490

Merged
merged 22 commits into from
Nov 15, 2022

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Jun 5, 2022

New Pull Request Checklist

Issue Description

Cloud errors thrown from beforeConnect and beforeSubscribe will now catch, just like errors from query.find

Related issue: #1489
Closes #1489

Approach

Throws error to subscription promise if error occurs

TODOs before merging

  • Add tests

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jun 5, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@mtrezza
Copy link
Member

mtrezza commented Jun 5, 2022

Since this is a breaking change, how would you like to proceed - add a temporary Parse Server option (or maybe there is an alternative?) or wait to merge until Parse Server 6?

@dblythy
Copy link
Member Author

dblythy commented Jun 5, 2022

Which approach do you think is best? It’s not a major issue so perhaps we can wait

@mtrezza
Copy link
Member

mtrezza commented Jun 5, 2022

Can you think of a way to support both for a while without making it a breaking change or adding a Parse Server option? Otherwise up to you, whatever you think is best.

Should we add this as a feature or fix? The issue is currently classified as feature.

@mtrezza mtrezza changed the title fix: allow Parse.Cloud.beforeSubscribe rejections to be caught by query.subscribe fix: allow Parse.Cloud.beforeSubscribe rejections to be caught by ParseQuery.subscribe Jun 5, 2022
@dplewis
Copy link
Member

dplewis commented Nov 10, 2022

@dblythy @mtrezza Can we get this into the 4.0.0 release?

@mtrezza
Copy link
Member

mtrezza commented Nov 10, 2022

That would be good

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title fix: allow Parse.Cloud.beforeSubscribe rejections to be caught by ParseQuery.subscribe fix: Allow Parse.Cloud.beforeSubscribe rejections to be caught by ParseQuery.subscribe Nov 10, 2022
@mtrezza mtrezza added the state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message label Nov 10, 2022
@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Base: 99.93% // Head: 99.89% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (e062596) compared to base (bc04b4b).
Patch coverage: 81.81% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #1490      +/-   ##
==========================================
- Coverage   99.93%   99.89%   -0.04%     
==========================================
  Files          61       61              
  Lines        5966     5973       +7     
  Branches     1366     1367       +1     
==========================================
+ Hits         5962     5967       +5     
- Misses          4        6       +2     
Impacted Files Coverage Δ
src/LiveQueryClient.js 98.99% <81.81%> (-1.01%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dblythy dblythy requested a review from a team November 14, 2022 11:23
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can be best explain the "breaking" aspect of this change?

Calling Parse.Query.subscribe() will now return a rejected promise if an error is thrown in Cloud Code Triggers beforeConnect or beforeSubscribe; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an error.on listener to handle these errors.

Does that make sense?

@dblythy
Copy link
Member Author

dblythy commented Nov 14, 2022

Yes, that sounds good to me

@mtrezza mtrezza changed the title fix: Allow Parse.Cloud.beforeSubscribe rejections to be caught by ParseQuery.subscribe fix: Error in Cloud Code Triggers beforeConnect or beforeSubscribe does not reject promise of Parse.Query.subscribe() Nov 15, 2022
@mtrezza mtrezza changed the title fix: Error in Cloud Code Triggers beforeConnect or beforeSubscribe does not reject promise of Parse.Query.subscribe() fix: Parse.Query.subscribe() does not return a rejected promise on error in Cloud Code Triggers beforeConnect or beforeSubscribe Nov 15, 2022
@mtrezza mtrezza merged commit 96d7174 into parse-community:alpha Nov 15, 2022
parseplatformorg pushed a commit that referenced this pull request Nov 15, 2022
# [4.0.0-alpha.2](4.0.0-alpha.1...4.0.0-alpha.2) (2022-11-15)

### Bug Fixes

* `Parse.Query.subscribe()` does not return a rejected promise on error in Cloud Code Triggers `beforeConnect` or `beforeSubscribe` ([#1490](#1490)) ([96d7174](96d7174))

### BREAKING CHANGES

* Calling `Parse.Query.subscribe()` will now return a rejected promise if an error is thrown in Cloud Code Triggers `beforeConnect` or `beforeSubscribe`; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an `error.on` listener to handle these errors (#1490) ([96d7174](96d7174))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0-alpha.2

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Nov 15, 2022
@parse-github-assistant
Copy link

The label state:released-alpha cannot be used in combination with state:breaking.

@parse-github-assistant parse-github-assistant bot removed the state:released-alpha Released as alpha version label Nov 15, 2022
@dblythy dblythy deleted the catch-errors branch November 15, 2022 03:21
mtrezza pushed a commit to mtrezza/Parse-SDK-JS that referenced this pull request Jan 22, 2023
…error in Cloud Code Triggers `beforeConnect` or `beforeSubscribe` (parse-community#1490)

BREAKING CHANGE: Calling `Parse.Query.subscribe()` will now return a rejected promise if an error is thrown in Cloud Code Triggers `beforeConnect` or `beforeSubscribe`; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an `error.on` listener to handle these errors (parse-community#1490)
mtrezza pushed a commit to mtrezza/Parse-SDK-JS that referenced this pull request Jan 22, 2023
# [4.0.0-alpha.2](parse-community/Parse-SDK-JS@4.0.0-alpha.1...4.0.0-alpha.2) (2022-11-15)

### Bug Fixes

* `Parse.Query.subscribe()` does not return a rejected promise on error in Cloud Code Triggers `beforeConnect` or `beforeSubscribe` ([parse-community#1490](parse-community#1490)) ([96d7174](parse-community@96d7174))

### BREAKING CHANGES

* Calling `Parse.Query.subscribe()` will now return a rejected promise if an error is thrown in Cloud Code Triggers `beforeConnect` or `beforeSubscribe`; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an `error.on` listener to handle these errors (parse-community#1490) ([96d7174](96d7174))
parseplatformorg pushed a commit that referenced this pull request Jan 23, 2023
# [4.0.0-beta.1](3.5.1...4.0.0-beta.1) (2023-01-23)

### Bug Fixes

* `Parse.Query.subscribe()` does not return a rejected promise on error in Cloud Code Triggers `beforeConnect` or `beforeSubscribe` ([#1490](#1490)) ([96d7174](96d7174))
* Remove support for Node <14 ([#1603](#1603)) ([bc04b4b](bc04b4b))

### Features

* Add Node 16 and 18 support ([#1598](#1598)) ([2c79a31](2c79a31))
* Add node 19 support ([8ed0fab](8ed0fab))
* Add Node 19 support ([#1643](#1643)) ([dfb5196](dfb5196))

### Performance Improvements

* Avoid CORS preflight request by removing upload listener when not used ([#1610](#1610)) ([6125419](6125419))

### BREAKING CHANGES

* Calling `Parse.Query.subscribe()` will now return a rejected promise if an error is thrown in Cloud Code Triggers `beforeConnect` or `beforeSubscribe`; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an `error.on` listener to handle these errors (#1490) ([96d7174](96d7174))
* This release removes support for Node versions <14 ([bc04b4b](bc04b4b))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jan 23, 2023
parseplatformorg pushed a commit that referenced this pull request Jan 23, 2023
# [4.0.0](3.5.1...4.0.0) (2023-01-23)

### Bug Fixes

* `Parse.Query.subscribe()` does not return a rejected promise on error in Cloud Code Triggers `beforeConnect` or `beforeSubscribe` ([#1490](#1490)) ([96d7174](96d7174))
* Remove support for Node <14 ([#1603](#1603)) ([bc04b4b](bc04b4b))

### Features

* Add Node 16 and 18 support ([#1598](#1598)) ([2c79a31](2c79a31))
* Add node 19 support ([8ed0fab](8ed0fab))
* Add Node 19 support ([#1643](#1643)) ([dfb5196](dfb5196))

### Performance Improvements

* Avoid CORS preflight request by removing upload listener when not used ([#1610](#1610)) ([6125419](6125419))

### BREAKING CHANGES

* Calling `Parse.Query.subscribe()` will now return a rejected promise if an error is thrown in Cloud Code Triggers `beforeConnect` or `beforeSubscribe`; in previous releases a resolved promise was returned, even if subscribing failed and it was necessary to create an `error.on` listener to handle these errors (#1490) ([96d7174](96d7174))
* This release removes support for Node versions <14 ([bc04b4b](bc04b4b))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 4.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:breaking Breaking change requires major version increment and `BREAKING CHANGE` commit message state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse Cloud beforeConnect and beforeSubscribe errors should throw in ParseQuery.subscribe
4 participants