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

Nested Fields Failing to Resolve for Mutations Due to Subscriptions Protection #2616

Closed
2 tasks done
matt-at-allera opened this issue Jun 11, 2024 · 5 comments
Closed
2 tasks done

Comments

@matt-at-allera
Copy link

matt-at-allera commented Jun 11, 2024

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

20.7 locally, 16.19.0 amplify build image

Amplify CLI Version

12.12.2 locally, 12.12.1 build setting

What operating system are you using?

Mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes that caused the issue. I manually changed the pipeline resolves that run for nested fields to verify the problematic block of code and fix.

Describe the bug

In our development environment, we are seeing the auth resolvers for nested models fail to resolve any of the nested fields. This is causing many errors of the form:

"Cannot return null for non-nullable type: 'ID' within parent 'Organization' (/createStation/Site/Organization/id)"

The issue exists only while mutating objects and when the mutation is specifying nested fields (Site / Organization in this case), and it has appeared to effect our graphQL api in it's entirety. Upon digging into why this might be the issue, I looked at the variance between our resolvers in this development environment and our staging environment. I found that all of the auth (...auth0Function) resolvers for the nested models contained this block of code at the beginning:

## [Start] Check if subscriptions is protected. **
#if( $util.defaultIfNull($ctx.source.get("__operation"), null) == "Mutation" )
  $util.qr($ctx.stash.put("deniedField", true))
#end
## [End] Check if subscriptions is protected. **

When this is removed from the resolver, the resolver runs as expected and the fields are successfully queried. My initial instinct is that this was intended to read Subscription instead of Mutation, but I do not know enough about what is going on here. I found this test file with the same verbiage. I couldn't locate the exact root of why the auth resolvers generate with this statement.

Expected behavior

Nested fields in a mutation should not be automatically denied from resolving.

Reproduction steps

I'm not positive what caused the problematic code block to generate in the auth resolvers. This is not appearing in our staging or production environments, even on the exact same revision head. It may be related to recent cli updates?

Project Identifier

No response

Log output

# Put your logs below this line


Additional information

No response

Before submitting, please confirm:

  • I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • I have removed any sensitive information from my code snippets and submission.
@matt-at-allera
Copy link
Author

matt-at-allera commented Jun 12, 2024

I ended up writing a Python script that utilizes the python sdk to automate removal of the subscriptions protection code block that is causing this issue (see attached zip)

> python3 remove_errant_subscription_protection.py
Removed subscriptions protection code block from ...auth0Function for xxx.xxx

I would still like to better understand why these code blocks were generated in the first place.

remove_errant_subscription_protection.py.zip

@NicholasAllen
Copy link

This sounds very similar to #2609 which was caused by a recent change: #2536

Frustratingly, the initial response: #2609 (comment)
suggests that this change in behaviour is entirely correct.

Given that it seems that quite a few of us have been relying upon this behaviour and it is now unexpectedly broken, I don't think that holds up very well.

@matt-at-allera
Copy link
Author

This sounds very similar to #2609 which was caused by a recent change: #2536

Frustratingly, the initial response: #2609 (comment) suggests that this change in behaviour is entirely correct.

Given that it seems that quite a few of us have been relying upon this behaviour and it is now unexpectedly broken, I don't think that holds up very well.

Thanks for sharing and tagging. I absolutely agree. If it is intended, the default case needs to at least be configurable.

@chrisbonifacio
Copy link
Member

Hi @matt-at-allera, thanks for raising this issue. As @NicholasAllen mentioned, this is a duplicate of #2609 and the behavior reported is currently the intended behavior. Please track #2609 for further updates.

Closing this issue as a duplicate for easier tracking.

@chrisbonifacio chrisbonifacio closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2024
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants