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

Lack of fail over for emitters. #50

Open
smentek opened this issue Jun 17, 2015 · 5 comments
Open

Lack of fail over for emitters. #50

smentek opened this issue Jun 17, 2015 · 5 comments
Assignees

Comments

@smentek
Copy link

smentek commented Jun 17, 2015

Lets say I want to track unstructEvent link:
$trucker->trackUnstructEvent($json, $context, $timestamp);

Now let say I want to do some actions in case it fails. There is no way to do that.

  • No Exception that could be catched
  • No false flag in return

Nothing. The same for all other calls.

It is like speed would be all we care about here and reliability is nothing.
But if speed is what we are focus on should we even write it in PHP? I would prefer slow but readable and safe solution...

After adding Exceptions please remove all 'print_r' from code. What do do with error messages that would be properly set in exceptions should be part of application that uses snowplow-tracker.

@alexanderdean
Copy link
Member

@smentek
Copy link
Author

smentek commented Jun 17, 2015

I'm afraid I did.

What is needed is:

if (!$trucker->trackUnstructEvent($json, $context, $timestamp)) {
Here save the the day!
}

or even better:

try {
$trucker->trackUnstructEvent($json, $context, $timestamp)
} catch(ExceptionSnowplowTracker $e) {
Here save the the day!

print_r(sprintf('%s - %s', "This is what went wrong: ", $e->getMessage()))
}

This: $debug parameter inside SyncEmitter::__construct( whatever here, $debug) should be removed. I do not see any added value from having debugger as part of code going on production.

@alexanderdean
Copy link
Member

Thanks for the feedback - scheduled!

@oguzhanunlu oguzhanunlu removed this from the Version 0.3.0 milestone May 24, 2019
@brenton-vidcorp
Copy link

brenton-vidcorp commented Feb 26, 2021

I've just been looking into how to gracefully handle failures when using POST and buffer size/batch of 50 events on 0.3.1.

Looks like I could so something like
$buffer = $this->emitter->returnBuffer(); $result = $this->emitter->send($buffer);

and do something if the result != true

But then I won't be able to reset the buffer... so any more events I add via track() will cause a flushEvents to trigger and send the events again.

Unless I'm missing something... The debug flag and using returnRequestResults doesn't seem to do be returning anything despite Sync POST Request Failed: Requests_Exception: cURL error 28: Connection timed out appearing in the debug log. Nope that was my bad

I think that if flushEvents or send is getting called somewhere and it fails to flush... that exception should be bubbling out so we can handle and retry/requeue whatever (this would also cause the exception to be generated in trackUnstructEvent.

@adatzer
Copy link
Contributor

adatzer commented Mar 4, 2021

Hello @brenton-vidcorp and @smentek ! Thank you very much for your feedback!
The lack of fail over is indeed something we have considered and we will rather build a proper buffer and retry mechanism in the php-tracker itself, instead of using exceptions.
Also, return values are a somewhat fragile way of information passing in this context, but it would be great if you could share your ideas along so as to understand your use cases better.
What do you think?

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

6 participants