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

feat(agent): return SerializationError instead of aborting #121

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Apr 26, 2024

When serializing we let the SerializationError go straight to the user and we should do the same on deserializing instead of returning a grpc error.

Copy link

linear bot commented Apr 26, 2024

@efiop efiop force-pushed the ruslan/fea-2353-better-error-messages-handle-internal-errors-gracefully branch from ef30ef9 to 8b4e8af Compare April 26, 2024 17:42
@efiop efiop force-pushed the ruslan/fea-2353-better-error-messages-handle-internal-errors-gracefully branch from 8b4e8af to d43e6f8 Compare April 26, 2024 17:47
@efiop efiop changed the title [WIP] feat(agent): return more exceptions [WIP] feat(agent): return SerializationError instead of aborting Apr 26, 2024
@efiop
Copy link
Contributor Author

efiop commented Apr 26, 2024

Testing with unreleased version is hell in fal-isolate-cloud (e.g. poetry seems to be having troubles with dependency resolution when updating the lock), so I'll just crank out a new release here (we lock particular version everywhere, so shouldn't be an issue).

@efiop efiop changed the title [WIP] feat(agent): return SerializationError instead of aborting feat(agent): return SerializationError instead of aborting Apr 26, 2024
@efiop efiop marked this pull request as ready for review April 26, 2024 18:00
@efiop efiop merged commit 042c78f into main Apr 26, 2024
6 checks passed
@efiop efiop deleted the ruslan/fea-2353-better-error-messages-handle-internal-errors-gracefully branch April 26, 2024 18:00
Copy link
Member

@chamini2 chamini2 left a comment

Choose a reason for hiding this comment

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

nice

efiop added a commit that referenced this pull request May 21, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 22, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 22, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 23, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 23, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 23, 2024
We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
efiop added a commit that referenced this pull request May 23, 2024
* fix(agent): use context.abort() instead of returning

We stopped always yielding stuff unconditionally, so now if you get
an error and use our own `abort_with_msg` we'll get an error
iterating responses, as we never yield anything and just return `None`.
So instead just use proper grpc `ServicerContext.abort()` method to
raise an exception instead.

* fix(agent): log exception instead of returning (revert #121)

We need to keep backward compatibility with older clients and they will
have trouble unpickling a new exception, as we are sending these out
pickled by reference and not by value (solving that would require
serialization backend specific actions, which I would rather avoid
for now to unblock a release).
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.

2 participants