Improve Excel2SBOL conversion error reporting#357
Conversation
| return jsonify({ | ||
| "status": "error", | ||
| "error": { | ||
| "code": classified["code"], | ||
| "message": classified["message"], | ||
| "hint": classified["hint"], | ||
| "details": redact_sensitive_values(str(exc)), | ||
| "technical_details": { | ||
| "terminal_output": redact_sensitive_values(terminal_output), | ||
| "traceback": redact_sensitive_values(traceback.format_exc()), | ||
| }, | ||
| }, | ||
| }), classified["status"] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7d04f14ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except AttributeError as e: | ||
| os.remove(metadata_path) | ||
| return jsonify({"error": str(e)}), 400 | ||
| with contextlib.redirect_stdout(stdout_buffer), contextlib.redirect_stderr(stderr_buffer): |
There was a problem hiding this comment.
Avoid process-wide stdout redirection during uploads
When the backend runs with a threaded Flask/Wsgi worker, contextlib.redirect_stdout/redirect_stderr replaces the process-wide sys.stdout and sys.stderr for the entire duration of this potentially long XDC upload. Any concurrent request or background task that prints/logs to these streams during that window will be captured into this user's technical_details response, which can leak unrelated output and also hide it from server logs. Prefer a conversion-local logging/capture mechanism, or avoid returning global stdout/stderr capture from a request handler.
Useful? React with 👍 / 👎.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://witty-hill-08172a210-357.centralus.5.azurestaticapps.net |
Motivation
Description
contextlib.redirect_stdout/redirect_stderr, classify common exceptions (FileNotFoundError,KeyError,ValueError, fallback) and return a single JSON error payload withstatus: "error",error.code,error.message,error.hint,error.details, anderror.technical_detailscontainingterminal_outputandtraceback; add aredact_sensitive_valueshelper to replace configured env values with[REDACTED].app.logger.exceptionand preserve existing success responses unchanged.ConversionErrorrenderer andshowUploadErrorNotification/showConversionErrorNotificationhelpers that initially show only the friendlymessageandhintwith a localMore/Hide detailstoggle that revealstechnical_detailsfrom the already-returned response without further network calls.ValueErrorfrom the XDC constructor to assert the JSON shape, presence oftechnical_details, and that redaction removed the secret value.Testing
python -m py_compile backend/sbs_server/app/views.pyto validate syntax, which succeeded.pytest backend/tests/test_excel2sbol_errors.py) but collection failed in this environment due to missing runtime dependency (ModuleNotFoundError: No module named 'flask'), so the test could not be executed here.npm run build(Vite) which completed successfully (build warnings about large chunks but no errors).Codex Task