Skip to content

POWERSYNC prototype: add migration to create postgresql publication#1747

Draft
Dieterbe wants to merge 40 commits intomasterfrom
powersync
Draft

POWERSYNC prototype: add migration to create postgresql publication#1747
Dieterbe wants to merge 40 commits intomasterfrom
powersync

Conversation

@Dieterbe
Copy link
Copy Markdown
Contributor

@Dieterbe Dieterbe commented Aug 2, 2024

No description provided.

Comment thread wger/nutrition/migrations/0025_create_publication.py Outdated
This is needed since the move to vite on the react side
This wasn't working well with when used over the API
This still needs to be clean up (specially the key generation, at the moment we
use a new dependency, but all this can be also done with the library we currently
use)
Comment thread wger/core/api/views.py
try:
return JsonResponse({'token': token, 'powersync_url': settings.POWERSYNC_URL}, status=200)
except Exception as e:
return JsonResponse({'error': str(e)}, status=500)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix

AI 3 days ago

Use standard secure exception handling for API endpoints:

  • Keep detailed error information in server logs.
  • Return only a generic error message to clients.
  • Avoid changing endpoint behavior beyond sanitizing error output.

Best fix in this snippet:

  • In wger/core/api/views.py, update the except Exception as e block in get_powersync_token (around lines 491–492).
  • Replace return JsonResponse({'error': str(e)}, status=500) with:
    • a logger.exception(...) call (captures stack trace server-side),
    • a generic JsonResponse({'error': 'An internal error has occurred.'}, status=500).

No new imports are needed because logging is already imported and logger is already used in this file context.

Suggested changeset 1
wger/core/api/views.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/wger/core/api/views.py b/wger/core/api/views.py
--- a/wger/core/api/views.py
+++ b/wger/core/api/views.py
@@ -488,8 +488,9 @@
 
     try:
         return JsonResponse({'token': token, 'powersync_url': settings.POWERSYNC_URL}, status=200)
-    except Exception as e:
-        return JsonResponse({'error': str(e)}, status=500)
+    except Exception:
+        logger.exception('Error returning PowerSync token response')
+        return JsonResponse({'error': 'An internal error has occurred.'}, status=500)
 
 
 @api_view()
EOF
@@ -488,8 +488,9 @@

try:
return JsonResponse({'token': token, 'powersync_url': settings.POWERSYNC_URL}, status=200)
except Exception as e:
return JsonResponse({'error': str(e)}, status=500)
except Exception:
logger.exception('Error returning PowerSync token response')
return JsonResponse({'error': 'An internal error has occurred.'}, status=500)


@api_view()
Copilot is powered by AI and may make mistakes. Always verify output.
rolandgeider and others added 12 commits September 8, 2024 21:04
most convenient and should be just fine
This is only needed because powersync can't handle joins
- move create publication to core
- use ivm entries for the nutrition publications
# Conflicts:
#	wger/core/static/react/main.js
This is needed in the flutter app to properly be able to set IDs on device, since
the regular IDs can only be set by postgres.
@real-yfprojects
Copy link
Copy Markdown

Hey there. What is the current state of this, which steps are needed going forward and how can I help (as a python dev)?

Comment thread wger/core/api/views.py
return JsonResponse({'error': f'Unknown table: {table}'}, status=200)
except Exception as e:
logger.exception(f'Error processing PowerSync data for table {table}')
return JsonResponse({'error': str(e)}, status=200)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix

AI 3 days ago

Return a generic client-facing error message instead of str(e), while keeping detailed diagnostics in server logs.

Best fix in this snippet:

  • In wger/core/api/views.py, inside upload_powersync_data’s except Exception as e: block (around lines 573–575), keep logger.exception(...) as-is (it preserves debugging detail server-side).
  • Replace return JsonResponse({'error': str(e)}, status=200) with a generic message such as {'error': 'An internal error has occurred.'}.
  • No import changes are required.

This preserves existing behavior (same response structure and status code) while preventing exception-detail disclosure.

Suggested changeset 1
wger/core/api/views.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/wger/core/api/views.py b/wger/core/api/views.py
--- a/wger/core/api/views.py
+++ b/wger/core/api/views.py
@@ -572,6 +572,6 @@
                 return JsonResponse({'error': f'Unknown table: {table}'}, status=200)
     except Exception as e:
         logger.exception(f'Error processing PowerSync data for table {table}')
-        return JsonResponse({'error': str(e)}, status=200)
+        return JsonResponse({'error': 'An internal error has occurred.'}, status=200)
 
     return JsonResponse({'status': 'ok!'}, status=200)
EOF
@@ -572,6 +572,6 @@
return JsonResponse({'error': f'Unknown table: {table}'}, status=200)
except Exception as e:
logger.exception(f'Error processing PowerSync data for table {table}')
return JsonResponse({'error': str(e)}, status=200)
return JsonResponse({'error': 'An internal error has occurred.'}, status=200)

return JsonResponse({'status': 'ok!'}, status=200)
Copilot is powered by AI and may make mistakes. Always verify output.
@rolandgeider
Copy link
Copy Markdown
Member

hey @real-yfprojects, sorry for the late reply, hadn't seen your message.

Most of the work will need to happen on the flutter side of things, but here there will be some cleaning up needed to do before we can really release this (we also moved some models to use uuids as the PK so we can generate them client side as well without having to do mappings back and forth, so possibly something among those lines as well)

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

Successfully merging this pull request may close these issues.

4 participants