Conversation
There was a problem hiding this comment.
It's likely a bit unoptimized to allocate the string every time but it should still be very little work in comparison to actually running pathfinding or mapmatching or similar.
There was a problem hiding this comment.
Is valhalla_exception_t appropriate here instead of std::runtime_error?
There was a problem hiding this comment.
Looking at other exceptions thrown here, seems like valhalla_exception_t is the right choice.
There was a problem hiding this comment.
Aye, removed the runtime error for valhalla_exception_t
Example log: ``` 2020/07/17 18:31:10.483608 [INFO] Got Loki Request 0 2020/07/17 18:31:10.486783 [ANALYTICS] locations_count::2 2020/07/17 18:31:10.486822 [ANALYTICS] costing_type::auto 2020/07/17 18:31:10.486859 [ANALYTICS] total_location_distance::7.487539km 2020/07/17 18:31:10.622645 [INFO] Got Thor Request 0 2020/07/17 18:31:10.622708 [ERROR] Failed parsing pbf in Thor::Worker 2020/07/17 18:31:10.622739 [ANALYTICS] 400::Failed parsing pbf in Thor::Worker request_id=0 0 2020/07/17 18:31:10.622851 400 239 ```
| std::string serialize_to_pbf(Api& request) { | ||
| std::string buf; | ||
| if (!request.SerializeToString(&buf)) { | ||
| LOG_ERROR("Failed serializing to pbf in Thor::Worker - trace_route"); |
There was a problem hiding this comment.
We're calling this helper from optimized_route, route & trace_route. So maybe we can be remove the trace_route part of the error message since it could be generated for other options.
mandeepsandhu
left a comment
There was a problem hiding this comment.
Just 1 minor comment. Otherwise looks good to me 👍
|
Pushed an update to the osrm error message for the 401 valhalla exception 6037949 |
* 'master' of github.com:valhalla/valhalla: Make tile building reproducible (#2480) Fix install-script for ubuntu 18.04 (#2306) (#2486) nit(git): Configures changelog to resolve conflicts with union strategy (#2489) feat(traffictile.h): Adds versioning checks (#2484) Fix dereferencing of end for std::lower_bound and possible UB (#2488) Minor fixes to tests (#2483) nit: Missing changelog entry (#2478) Safiefy protobuf (#2477)
PR adds checks on protobuf serializing and de-serializing in the worker, and logs request-id of failing requests.
Produces log: