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: await Sentry. flush() on Vercel #2

Merged
merged 6 commits into from
Sep 26, 2021

Conversation

natterstefan
Copy link
Owner

@natterstefan natterstefan commented Jun 30, 2021

Debugging

With Try Catch: ✅

Log on Vercel

image

Without Try Catch: ❌

Log on Vercel

image

Vercel Logs

The false log on Vercel comes from the await Sentry.flush(2000)

image

Promise.reject vs. Promise.resolve

If you provide a timeout and the queue takes longer to drain the promise returns false.

// src: https://github.com/getsentry/sentry-javascript/blob/a732ae830b613f0227330b93038695ad678ed591/packages/node/src/sdk.ts#L153

/**
 * A promise that resolves when all current events have been sent.
 * If you provide a timeout and the queue takes longer to drain the promise returns false.
 *
 * @param timeout Maximum time in ms the client should wait.
 */
export async function flush(timeout?: number): Promise<boolean> {
  const client = getCurrentHub().getClient<NodeClient>();
  if (client) {
    return client.flush(timeout);
  }
  return Promise.reject(false);
}

They do not resolve but reject the promise. The comment was misleading...

This try/catch solution was mentioned here as well.

Also, the with-sentry example does not indicate that you should try/catch await Sentry.flush as well (https://github.com/vercel/next.js/blob/9ab916ac99eb200a478c8d5ffe94b6ec9c0a315b/examples/with-sentry/pages/api/test4.js#L12).

@natterstefan natterstefan added bug Something isn't working work in progress labels Jun 30, 2021
@natterstefan natterstefan self-assigned this Jun 30, 2021
@vercel
Copy link

vercel bot commented Jun 30, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/natterstefan/next-with-sentry/C94cSJ7ppk3AUesFmuwwefPn9yrw
✅ Preview: https://next-with-sentry-git-fix-202106-internal-se-ed38d6-natterstefan.vercel.app

@natterstefan
Copy link
Owner Author

natterstefan commented Jun 30, 2021

Update 2021-06-30

I created a PR in the next.js repo with the try/catch solution: vercel/next.js#26767. While I was creating it I found this comment getsentry/sentry-javascript#3643 (comment) in an issue describing my problem with Sentry.flush. Interestingly withSentry handles it silently (mentioned here).

The maintainers of @sentry/nextjs catch it as well: getsentry/sentry-javascript@6c1b4bd#diff-1d2c03eeaa78dd1674379a1628e39b2026cb775d32aea72a366702db9aba1325 😅.

And let's see what happens here getsentry/sentry-javascript#3746.

@natterstefan natterstefan changed the title chore: debug await Sentry. flush on Vercel fix: await Sentry. flush() on Vercel Sep 26, 2021
@natterstefan natterstefan marked this pull request as ready for review September 26, 2021 13:23
@natterstefan
Copy link
Owner Author

natterstefan commented Sep 26, 2021

I can confirm, that "@sentry/nextjs": "6.13.2" (as suggested here) resolves the issue (f768137, also with next@11 918f374)

Demo URL: https://next-with-sentry-31rtz63z3-natterstefan.vercel.app/api/test4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant