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.

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

ENH: Log errors to logger in addition to raising exceptions #108

Merged
merged 4 commits into from Jun 5, 2023

Conversation

brendan-ward
Copy link
Collaborator

Previously, most errors were not logged to the logger (server-side) and instead only returned in the response to the client, which made it difficult to debug when the errors were ultimately related to the server (e.g., missing an expected tileset)

Comment on lines -74 to +75
throw new Error(`Could not normalize Mapbox source URL: ${url}\n${e}`)
const msg = `Could not normalize Mapbox source URL: ${url}\n${e}`
logger.error(msg)
throw new Error(msg)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume there's no simple way to catch and log errors at a higher level?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no. Commit 42184b3 fixed the error logging at the server level (server must be started in verbose mode via -v); previously it was it was not passing back the string error message correctly. Fixing this allowed us to reduce the double-logging in the server tier.

Within the renderer it is a bit of a different story: there are some things that we can propagate back up via exception handling (by throwing errors) whereas in other places these are outside the main control flow. For example, we cannot raise exceptions based on style parsing error messages / warnings passed back by maplibre, since these happen outside the normal control flow (this is a known issue).

I did remove some of the unnecessary log statements from renderer related to validating inputs; those are already validated in the server tier, and when using the NodeJS API directly it is OK for those to simply raise errors.

@brendan-ward brendan-ward merged commit 9865eff into main Jun 5, 2023
6 of 7 checks passed
@brendan-ward brendan-ward deleted the log_errors branch June 5, 2023 18:00
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.

None yet

2 participants