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

Better adaptor errors #583

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from
Draft

Better adaptor errors #583

wants to merge 34 commits into from

Conversation

josephjclark
Copy link
Collaborator

Maybe closes #582

image

How am I going to test this?

Is the stacktrace really useful in lightning? Can we control this any how?

Is it really appropriate to graft the stack trace onto the error? Maybe it's better to return the stack to lightning and let lightning decide what to do.

This cause problems with the worker because errors get  flattened to {}, and also we have to double parse.

Now the logger will just emit whatever it logged to whatever the log emmiter is, so JSON stays as JSON. Which is good, but it no longer guarantees it'll be serializable
The logger always sends raw json, but the log message is stringified by the engine, and rebuilt by the worker before sending to lightning

this last bit needs work but its better
They were neglecting to parse the strings sent out by the new json logger
It was secretly failing under the hood
@josephjclark josephjclark changed the base branch from main to release/next February 1, 2024 11:56
@josephjclark
Copy link
Collaborator Author

I am not going to add this to the 1.0 branch - although it may get patched in shortly after

Base automatically changed from release/next to main February 8, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Icebox
Development

Successfully merging this pull request may close these issues.

CLI: Bad error output for http
1 participant