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

Correct pattern for flushing a (Sentry) event queue from middleware #519

Closed
rpinna opened this issue May 16, 2020 · 1 comment
Closed

Correct pattern for flushing a (Sentry) event queue from middleware #519

rpinna opened this issue May 16, 2020 · 1 comment

Comments

@rpinna
Copy link

rpinna commented May 16, 2020

[Question]

Thank you for the great work on this package, I just integrated it and it went very smoothly.

We use Sentry for crash reporting. A natural use case for a middleware is to log all exceptions and rejected promises to Sentry.

I'm not quite sure how to handle the requirement to flush the Sentry event queue. You can refer to this issue, but the TL;DR is that it's possible to drop Sentry events from a lambda function if you don't make sure that its background event queue is drained. This isn't an edge case, it happens regularly if you don't protect against it.

This is what I have for the middleware right now. Could you take a look at the two calls to sentry.flush and advise if this is the correct pattern?

If this is of interest, please let me know if you would like me to submit a PR.

import middy from '@middy/core';

interface Breadcrumb {
  message?: string;
  data?: {
    [key: string]: any;
  };
}

interface SentryLike {
  addBreadcrumb(breadcrumb: Breadcrumb): void;
  captureException(exception: any): string;
  flush(timeout?: number): Promise<boolean>;
}

interface Config {
  sentry: SentryLike;
  injectInContext?: boolean;
}
export function sentryMiddleware<TEvent = any, TResponse = any>(
  config: Config,
): middy.MiddlewareObject<TEvent, TResponse> {
  const { injectInContext, sentry } = config;
  return {
    before: (handler: any, next: any) => {
      if (injectInContext !== false) {
        handler.context.sentry = sentry;
      }
      return next();
    },
    after: async () => {
      await sentry.flush();
    },
    onError: async (handler: any) => {
      const { error, event } = handler;
      sentry.addBreadcrumb({
        message: 'lambda event',
        data: event,
      });
      sentry.captureException(error);
      await sentry.flush();
      return error;
    },
  };
}
@willfarrell
Copy link
Member

That pattern looks right to me. Because this depends on a 3rd party service we likely won't add into core at this time. I'd be happy to approve a PR to add it to the community middleware list.

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

No branches or pull requests

2 participants