-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add optional crash dumping on ConnectionError #1077
Changes from all commits
c0f3745
e6cbc10
ea9682b
1e12160
1830cf0
69f02cc
4903f7a
9e39b7a
47a16dc
e5d1415
d946180
f3d6aa2
b6fd11b
5c6888b
9dd556c
e41f3a1
feb769c
5af0b29
e6be326
61772b9
8bfe9a5
6743172
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
"""Client for interacting with the LangSmith API. | ||
Check notice on line 1 in python/langsmith/client.py GitHub Actions / benchmarkBenchmark results
Check notice on line 1 in python/langsmith/client.py GitHub Actions / benchmarkComparison against main
|
||
|
||
Use the client to customize API keys / workspace ocnnections, SSl certs, | ||
etc. for tracing. | ||
|
@@ -20,6 +20,7 @@ | |
import datetime | ||
import decimal | ||
import functools | ||
import gzip | ||
import importlib | ||
import importlib.metadata | ||
import io | ||
|
@@ -931,6 +932,43 @@ | |
filler = "*" * (max(0, len(api_key) - 7)) | ||
masked_api_key = f"{prefix}{filler}{suffix}" | ||
|
||
debug_crash_dump_file = ls_utils.get_env_var( | ||
"LANGSMITH_DEBUG_CRASH_DUMP" | ||
) | ||
if debug_crash_dump_file is not None and e.request is not None: | ||
try: | ||
headers = e.request.headers.copy() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hinthornw Let me know if you agree with this approach. Made a shallow copy of the headers so we could get rid of the x-api-key, but log the rest of the headers. Manually added all of the fields which I think are relevant here. This is to prevent us accidentally logging other sensitive fields (if any). |
||
headers.pop("x-api-key", None) | ||
request_to_log = { | ||
"method": e.request.method, | ||
"url": e.request.url, | ||
"headers": headers, | ||
"body": ( | ||
e.request.body | ||
if isinstance(e.request, requests.PreparedRequest) | ||
else None | ||
), | ||
"data": ( | ||
e.request.data | ||
if isinstance(e.request, requests.Request) | ||
else None | ||
), | ||
"json": ( | ||
e.request.json | ||
if isinstance(e.request, requests.Request) | ||
else None | ||
), | ||
} | ||
with gzip.open( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you easily open a gzip file that has been appended to multiple times? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just looked into it a bit, it is a bit of a pain to read from multiple streams. Do you think an acceptable alternative is to write to a file with a certain timestamp appended to the file path? We can probably turn the env variable back to a boolean, and then use a fixed static name like |
||
debug_crash_dump_file, "at", encoding="utf-8" | ||
) as f: | ||
json_data = json.dumps(request_to_log) | ||
f.write(json_data + "\n") | ||
recommendation += f" More info can be found in: {debug_crash_dump_file}." | ||
except Exception as write_error: | ||
recommendation += f" Encountered this error while trying to write to {debug_crash_dump_file}: {repr(write_error)}" | ||
continue | ||
|
||
raise ls_utils.LangSmithConnectionError( | ||
f"Connection error caused failure to {method} {pathname}" | ||
f" in LangSmith API. {recommendation}" | ||
|
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.
can we read env var once in global scope and then just use that here (reading env vars is expensive)