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

Add notes on usage in serverless environments #5380

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

dtran320
Copy link
Contributor

@dtran320 dtran320 commented Sep 19, 2023

Proposed changes

Added some documentation here in hopes of saving other developers the several hours I unfortunately spent thinking that just calling await closeAndFlush() would suffice for serverless environments—I needed to do the combination of both:

  1. Wrapping analytics.track calls in a Promise and awaiting it
  2. Instantiate new instances of analytics()— otherwise my big batch of events just never getting sent despite calling closeAndFlush() and having that return. I didn't add this to the documentation since I'm not sure if I'm doing this correctly— any feedback would be appreciated. Happy to update the documentation with this additional information if this is actually necessary. The README seems to suggest it's only necessary if using plugins.

Merge timing

ASAP to prevent any other developers upgrading from the deprecated analytics-node to this library that use it in a serverless environment and previously depended on await analytics.flush().

Related issues (optional)

- We need to wrap `.track` calls in a `Promise` in these environments, which isn't mentioned in this doc. 
- Link to the README in the repo for full details
@dtran320 dtran320 requested a review from a team as a code owner September 19, 2023 23:00
@dtran320 dtran320 requested review from forstisabella and removed request for a team September 19, 2023 23:00
Copy link
Contributor

@forstisabella forstisabella left a comment

Choose a reason for hiding this comment

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

two small changes and this'll be g2g!

Co-authored-by: forstisabella <92472883+forstisabella@users.noreply.github.com>
@forstisabella forstisabella merged commit 28b722f into segmentio:develop Sep 25, 2023
3 checks passed
@github-actions
Copy link
Contributor

Thank you for your contribution! Your pull request is merged, but may take a day or two to appear on the site.

@dtran320 dtran320 deleted the patch-1 branch September 26, 2023 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants