Fix crash in /requests/report/wait by removing missing requestReports #3564
Conversation
Signed-off-by: iGufrankhan <gufrankhankab123@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in the /requests/report/wait/<id> endpoint caused by calling an undefined requestReports function. The fix removes the erroneous function call and safely returns data from the report_requests dictionary.
Changes:
- Added import for
report_requestsfromaugur.api.view.init - Removed call to undefined
requestReports(id)function - Changed dictionary access from
report_requests[id]toreport_requests.get(id, {})to prevent KeyError
| def wait_for_report_request(id): | ||
| requestReports(id) | ||
| return jsonify(report_requests[id]) | ||
| return jsonify(report_requests.get(id, {})) |
There was a problem hiding this comment.
Returning an empty dictionary when the report ID is not found may not be appropriate API design. Clients cannot distinguish between a missing report and an actual empty report result. Consider returning a 404 status code or an error response when the report ID doesn't exist in report_requests, which would provide clearer feedback to API consumers.
| return jsonify(report_requests.get(id, {})) | |
| if id not in report_requests: | |
| return jsonify({ | |
| "error": "Report request not found", | |
| "report_id": id | |
| }), 404 | |
| return jsonify(report_requests[id]) |
| def wait_for_report_request(id): | ||
| requestReports(id) | ||
| return jsonify(report_requests[id]) | ||
| return jsonify(report_requests.get(id, {})) |
There was a problem hiding this comment.
If this value doesnt exist, doesnt that mean we are always going to be returning an empty dict?
I wonder if its better to completely remove the endpoint in that case if all its ever going to do is return an empty dict
There was a problem hiding this comment.
@MoralCode at first i think so
that if it gone always return empty then why not completely remove this endpoint but
The route already exists and is being called by the UI
Removing it now could break the UI or other code that still expects it
I didn’t remove the endpoint in this PR because it may still be referenced by the UI , and I wanted to keep the fix minimal and safe
i agree for
removing is the best option lets me check if remove this endpoint
code is not gone break
let me first verify it
There was a problem hiding this comment.
MainProcess] secondary:7e4c9aa35c4348ea9d2282425d939051@c6c439d5bd54 ready.
augur-1 | 2026-01-12 02:39:27 c6c439d5bd54 augur.api.server[121] ERROR Exception on /repos/views/repo/1 [GET]
augur-1 | Traceback (most recent call last):
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 2073, in wsgi_app
augur-1 | response = self.full_dispatch_request()
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1518, in full_dispatch_request
augur-1 | rv = self.handle_user_exception(e)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask_cors/extension.py", line 178, in wrapped_function
augur-1 | return cors_after_request(app.make_response(f(*args, **kwargs)))
augur-1 | ^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1516, in full_dispatch_request
augur-1 | rv = self.dispatch_request()
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/app.py", line 1502, in dispatch_request
augur-1 | return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/api/view/routes.py", line 226, in repo_repo_view
augur-1 | return render_module("repo-info", title="Repo", repo=repo, repo_id=id)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/api/view/utils.py", line 224, in render_module
augur-1 | return render_template('index.j2', **args)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/templating.py", line 147, in render_template
augur-1 | return _render(
augur-1 | ^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/templating.py", line 128, in _render
augur-1 | rv = template.render(context)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/jinja2/environment.py", line 1291, in render
augur-1 | self.environment.handle_exception()
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/jinja2/environment.py", line 925, in handle_exception
augur-1 | raise rewrite_traceback_stack(source=source)
augur-1 | File "/augur/augur/templates/index.j2", line 44, in top-level template code
augur-1 | {% include '%s.j2' % body ignore missing %}
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/augur/templates/repo-info.j2", line 23, in top-level template code
augur-1 | $.getJSON("{{ url_for('wait_for_report_request', id=repo_id) }}", function(image_data) {
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/helpers.py", line 338, in url_for
augur-1 | return appctx.app.handle_url_build_error(error, endpoint, values)
augur-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/flask/helpers.py", line 325, in url_for
augur-1 | rv = url_adapter.build(
augur-1 | ^^^^^^^^^^^^^^^^^^
augur-1 | File "/augur/.venv/lib/python3.11/site-packages/werkzeug/routing.py", line 2315, in build
augur-1 | raise BuildError(endpoint, values, method, self)
augur-1 | werkzeug.routing.BuildError: Could not build url for endpoint 'wait_for_report_request' with values ['id']. Did you mean 'repo_pull_requests_new' instead?
augur-keyman-1 | 2026-01-12 02:39:49,275 - KeyOrchestrator - INFO - ACK; for: 1
augur-keyman-1 | 2026-01-12 02:39:49,276 - KeyOrchestrator - INFO - ACK; for: 1
v View in Docker Desktop o View Config w Enable Watch
There was a problem hiding this comment.
see man
if remove the backend endpoint but
the frontend call this
flask give error if it is not present
so what do you think
There was a problem hiding this comment.
I recommend updating endpoint logic to return 404 error when report ID is not found.
| requestReports(id) | ||
| return jsonify(report_requests[id]) |
There was a problem hiding this comment.
can be updated like this -
if id in report_requests:
return jsonify(report_requests[id])
return jsonify({
"error": "Report request not found",
"report_id": id
}), 404
There was a problem hiding this comment.
thank for suggestion!That makes sense if report generation is still supported.
however as far i know that this he report generation pipeline(like requestReport) has been remove entrity
nothing populate report_requests
that why i changed minimal change so that fromted not get break
Let’s wait for @MoralCode to confirm, since I believe they were involved in removing the report pipeline.
|
my recommendation like if you want me to complete removal of this pipeline then backend+frontend removal second is my approach which we are currently also can you provide the context that why this api removed |
|
could you confirm if there is any active use case for this report generation feature? If not, I did support full removal one. |
4 participants
Description
Fix crash in /requests/report/wait/ caused by calling a missing function requestReports.
Remove the call and safely return data from report_requests.
This PR fixes #3563
Notes for Reviewers
requestReports was never defined or imported.
Endpoint now uses:
return jsonify(report_requests.get(id, {}))Prevents NameError and KeyError.
Signed commits