-
Notifications
You must be signed in to change notification settings - Fork 13
Require API key and handle server errors #13
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
base: master
Are you sure you want to change the base?
Conversation
If the env variable is required to function, we should error if it's not set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures the POLYGON_API_KEY
must be set before startup, adds basic error handling and resource cleanup in the server’s run
function, and updates documentation to reflect the failure mode when the key is missing.
- Enforce
POLYGON_API_KEY
presence and raise an error if it’s not set in bothserver.py
andentrypoint.py
. - Wrap the server launch in a
try/except/finally
to log errors and close the REST client. - Update README to note that the server will exit if the API key isn’t configured.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/mcp_polygon/server.py | Enforce API key, add try/except/finally around run , clean up client |
entrypoint.py | Enforce API key check on startup |
README.md | Document server exit behavior when API key is missing |
Comments suppressed due to low confidence (2)
src/mcp_polygon/server.py:11
- Add a unit test for server initialization to ensure it raises an EnvironmentError when POLYGON_API_KEY is not set.
raise EnvironmentError("POLYGON_API_KEY environment variable not set.")
entrypoint.py:9
- Consider adding a test to verify that
start_server()
exits with an EnvironmentError when POLYGON_API_KEY is missing.
raise EnvironmentError("POLYGON_API_KEY environment variable not set.")
except Exception as exc: | ||
print(f"Error running Polygon MCP server: {exc}") | ||
raise | ||
finally: | ||
try: | ||
polygon_client.close() | ||
except Exception as close_exc: | ||
print(f"Error closing Polygon REST client: {close_exc}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching a broad Exception may mask unexpected errors; consider catching more specific exceptions or allowing critical errors to propagate.
except Exception as exc: | |
print(f"Error running Polygon MCP server: {exc}") | |
raise | |
finally: | |
try: | |
polygon_client.close() | |
except Exception as close_exc: | |
print(f"Error closing Polygon REST client: {close_exc}") | |
except RuntimeError as exc: | |
print(f"Runtime error while running Polygon MCP server: {exc}") | |
raise | |
except ValueError as exc: | |
print(f"Value error while running Polygon MCP server: {exc}") | |
raise | |
finally: | |
try: | |
polygon_client.close() | |
except OSError as close_exc: | |
print(f"OS error while closing Polygon REST client: {close_exc}") |
Copilot uses AI. Check for mistakes.
print(f"Error running Polygon MCP server: {exc}") | ||
raise | ||
finally: | ||
try: | ||
polygon_client.close() | ||
except Exception as close_exc: | ||
print(f"Error closing Polygon REST client: {close_exc}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer using a logging framework instead of print statements for error reporting to ensure consistent log management and easier debugging.
print(f"Error running Polygon MCP server: {exc}") | |
raise | |
finally: | |
try: | |
polygon_client.close() | |
except Exception as close_exc: | |
print(f"Error closing Polygon REST client: {close_exc}") | |
logging.error(f"Error running Polygon MCP server: {exc}") | |
raise | |
finally: | |
try: | |
polygon_client.close() | |
except Exception as close_exc: | |
logging.error(f"Error closing Polygon REST client: {close_exc}") |
Copilot uses AI. Check for mistakes.
@@ -4,9 +4,9 @@ | |||
|
|||
# Ensure the server process doesn't exit immediately when run as an MCP server | |||
def start_server(): | |||
polygon_api_key = os.environ.get("POLYGON_API_KEY", "") | |||
polygon_api_key = os.environ.get("POLYGON_API_KEY") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Retrieval and validation of POLYGON_API_KEY is duplicated in server.py
; consider centralizing this logic in a shared helper function.
Copilot uses AI. Check for mistakes.
Summary
POLYGON_API_KEY
inserver
andentrypoint
run
Testing
python -m py_compile entrypoint.py src/mcp_polygon/server.py