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.

Sign up for GitHub

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

Jump to bottom

Better handle sqlite error logs #135

Closed
wonder-sk opened this issue Jul 12, 2021 · 2 comments
Closed

Better handle sqlite error logs #135

wonder-sk opened this issue Jul 12, 2021 · 2 comments
Assignees
@alexbruy
Labels
enhancement New feature or request

Comments

@wonder-sk
Copy link
Contributor

Currently, geodiff registers its logger in GEODIFF_init() as the global log callback:

sqlite3_config( SQLITE_CONFIG_LOG, _errorLogCallback );

This may cause issues:

  • when used in another app, this may be in conflict with the app's log config for sqlite
  • geodiff may capture also logs that were not triggered by geodiff, making the log output confusing
  • if there are multiple instances of geodiff created within a single process, errors may not end up in the correct logger

It should be possible to catch errors on per-connection level without registering global error callback: http://www.sqlite.org/c3ref/errcode.html
@wonder-sk wonder-sk added the enhancement New feature or request label Jul 12, 2021
wonder-sk added a commit to MerginMaps/work-packages that referenced this issue Jul 12, 2021
@wonder-sk
Fix a crash when working with many work packages
07c6e4a 
The crash was happening inside geodiff due to the fact that mergin
client was creating geodiff instances, setting logger callbacks
to temporary python functions, making it possible that geodiff
calls could crash when trying to log using invalid logger callback.

Related geodiff enhancements:
MerginMaps/geodiff#135
MerginMaps/geodiff#136
@PeterPetrik
Copy link
Contributor

best after #136 is implemented

@PeterPetrik PeterPetrik mentioned this issue Jun 2, 2022
Merged
@alexbruy alexbruy mentioned this issue Jun 13, 2022
Merged
@alexbruy
Copy link
Contributor

Should be covered by #173.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alexbruy alexbruy

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@wonder-sk @alexbruy @PeterPetrik