Skip to content

Flushing doesn't work with asynchronous transport #2000

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

Closed
1 task done
sheerun opened this issue Apr 6, 2019 · 2 comments
Closed
1 task done

Flushing doesn't work with asynchronous transport #2000

sheerun opened this issue Apr 6, 2019 · 2 comments

Comments

@sheerun
Copy link
Contributor

sheerun commented Apr 6, 2019

  • @sentry/node (5.0.5)

Sentry.flush() doesn't wait for events to be delivered by transport. This can result in dropped events in serverless environment like AWS lambda.

Reproduction:

const Sentry = require('@sentry/node')

function sleep(delay) {
  return new Promise(resolve => setTimeout(resolve, delay))
}

class FakeTransport extends Sentry.Transports.HTTPTransport {
  async sendEvent(event) {
    console.log('Started sending ' + event.event_id)
    await sleep(500)
    console.log('Finished sending ' + event.event_id)
    return { status: 200 }
  }
}

Sentry.init({
  dsn: 'https://[email protected]/1234',
  transport: FakeTransport,
  beforeSend: (event) => {
    console.log('Before send ' + event.event_id)
    return event
  }
})

async function main() {
  Sentry.captureException(new Error('test 3'))
  console.log('Before')
  await Sentry.flush(2000)
  console.log('After')
  await sleep(3000)
}

main()
Before
Before send 3879a904216343bcb7999f116e336797
Started sending 3879a904216343bcb7999f116e336797
After
Finished sending 3879a904216343bcb7999f116e336797
@HazAT
Copy link
Member

HazAT commented Apr 8, 2019

You need to add the request to the transport internal buffer so close is aware when requests are finished.

protected readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);

So if you implement your custom transport, you need to at least implement close which takes care of this.

@HazAT HazAT closed this as completed Apr 8, 2019
@sheerun
Copy link
Contributor Author

sheerun commented Apr 8, 2019

I don't want to implement custom transport. The issue is with default http / https transport. I've just provided you a way to reproduce this issue.

To reproduce this issue directly on upstream https implementation, please change in node_modules https transport sendEvent to:

    HTTPSTransport.prototype.sendEvent = function (event) {
        console.log('sending https')
        // ... rest
    };

and add console.log('resolved') near resolve() call in BaseTransport.prototype._sendWithModule of base transport.

The result are following logs:

Before
Before send 4f316abe03e94142bfa75e2c231b2ceb
sending https
After
Before send 5d6c78c88ea74e06ae721c9ace4cf04f
sending https
resolved
resolved

As you can see even default transports fails to fully send events after Sentry.flush() finishes.

Also: Why isn't promise returned from sendEvent automatically added to _buffer? It seems more sensitive than documenting internal _buffer thing.

Please reopen

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

No branches or pull requests

2 participants