[Functions] Pass placeholder app check tokens on error

[ncooke3]

Currently, the Functions SDK does not pass the App Check placeholder token for cases where the App Check integration fails to fetch an App Check token.

Before:

After:

[google-oss-bot]

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

[paulb777]

Could this be a breaking change for apps depending on the nil return?

[ncooke3]

Could this be a breaking change for apps depending on the nil return?

I don't think so. Here is my thinking: currently, returning nil means the App Check header won't be included. This currently happens when there is an App Check error. This PR will send the placeholder token for the App Check header in the cases where nil is returned. So, the net change will be a token is always sent. If the AppCheck integration is unenforced, there shouldn't be a difference since the presence of the token is not being enforced. If the AppCheck integration is enforced, there also shouldn't be a difference because no token or placeholder token are both not forms of valid tokens. For the backend, the difference will be it sees the placeholder token.

cc: @weixifan

[andrewheard]

Could this be a breaking change for apps depending on the nil return?

I don't think so. Here is my thinking: currently, returning nil means the App Check header won't be included. This currently happens when there is an App Check error. This PR will send the placeholder token for the App Check header in the cases where nil is returned. So, the net change will be a token is always sent. If the AppCheck integration is unenforced, there shouldn't be a difference since the presence of the token is not being enforced. If the AppCheck integration is enforced, there also shouldn't be a difference because no token or placeholder token are both not forms of valid tokens. For the backend, the difference will be it sees the placeholder token.

That's my understanding as well. Based on these docs https://firebase.google.com/docs/app-check/monitor-functions-metrics I think devs will start seeing INVALID instead of MISSING. From what I can tell, devs would be using functionality provided by the Cloud Functions SDK (https://firebase.google.com/docs/app-check/cloud-functions), not manually writing it themselves.

[andrewheard]

Thanks, Nick! This LGTM from a Swift perspective. I'll ping Victor on Chat just to double check if that there's no reason to hold off on merging.

[weixifan]

Thanks, Nick! This LGTM from a Swift perspective. I'll ping Victor on Chat just to double check if that there's no reason to hold off on merging.

Just as a future record of what we discussed: Nick's reasoning is correct; this is safe to merge. Thank you Nick for the fix!