Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

fix: avoid destructuring undefined timestamps#1575

Merged
schmidt-sebastian merged 2 commits intogoogleapis:masterfrom
Sahtair:patch-1
Aug 2, 2021
Merged

fix: avoid destructuring undefined timestamps#1575
schmidt-sebastian merged 2 commits intogoogleapis:masterfrom
Sahtair:patch-1

Conversation

@Sahtair
Copy link
Copy Markdown

@Sahtair Sahtair commented Aug 2, 2021

In https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/index.ts#L911 we call from proto with a potential undefined instead of an object. This causes an error in https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/timestamp.ts#L122 where it tries to destructure an undefined value. This problem causes triggers to fail before entering user defined handler.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1574 🦕

In `https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/index.ts#L911` we call from proto with a potential undefined instead of an object. This causes an error in `https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/timestamp.ts#L122` where it tries to destructure an undefined value. This problem causes triggers to fail before entering user defined handler.
@Sahtair Sahtair requested review from a team August 2, 2021 11:48
@google-cla
Copy link
Copy Markdown

google-cla bot commented Aug 2, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Aug 2, 2021
@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Aug 2, 2021
@Sahtair
Copy link
Copy Markdown
Author

Sahtair commented Aug 2, 2021

@googlebot I signed it!

@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Aug 2, 2021
Copy link
Copy Markdown
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

Thank you for sending this. We might be able to simplify this if you are keen :)

@schmidt-sebastian schmidt-sebastian added the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
Co-authored-by: Sebastian Schmidt <mrschmidt@google.com>
@Sahtair
Copy link
Copy Markdown
Author

Sahtair commented Aug 2, 2021

@schmidt-sebastian Yes I would agree we can simplify. I was not sure what the convention was so I did the verbose way :) Thank you for the review!

@schmidt-sebastian schmidt-sebastian changed the title Proposed fixed to avoid destructuring undefined fix: avoid destructuring undefined timestamps Aug 2, 2021
@schmidt-sebastian schmidt-sebastian added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
@schmidt-sebastian schmidt-sebastian added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 2, 2021
@bcoe bcoe added the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 2, 2021
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 2, 2021
@schmidt-sebastian schmidt-sebastian merged commit a61a24a into googleapis:master Aug 2, 2021
gcf-merge-on-green bot pushed a commit that referenced this pull request Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: firestore Issues related to the googleapis/nodejs-firestore API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trigger fails due to destructuring undefined

4 participants