Show context length error msg by yensung · Pull Request #262 · allenai/olmo-api

yensung · 2025-04-02T19:55:58Z

Closes: https://github.com/allenai/playground-issues-repo/issues/248

Improved error handling to show context length error msg

mtblanton · 2025-04-02T20:00:52Z

src/message/create_message_service.py

+        except ValueError as e:
+            yield format_message(message.MessageStreamError(reply.id, f"{e}", "Context length is too long"))


This would catch other ValueErrors right? I don't think we want to generalize every ValueError to a context length error.

Also, how can we inform the UI that this is a context length error so it can show a special message with this?

Any suggestion for the error name? Otherwise I'll use ValueError directly

ValueError may work but flask_pydantic_api may catch it? I'd like to catch this kind of thing before it gets to this point tbh.

You may be able to raise an UpstreamError here instead of yielding a MessageStreamError?

Also, how can we inform the UI that this is a context length error so it can show a special message with this?

The exception content is a string value. Are you suggesting parsing the string to determine whether it's a context length error?

ValueError may work but flask_pydantic_api may catch it?

How do you catch it in flask_pydantic_api? I only see they are all ValueError here

sorry, I meant that flask_pydantic_api has some handling around Pydantic types like this. If it's thrown directly it may do something special?

Are you suggesting parsing the string to determine whether it's a context length error?

There's nothing else in it? I figured it may have more.

We may need to add request validation to have this show up nicely in the UI then. If that's more work than we want to take on right now we can put it off to another time.

It doesn't look like a pydantic error type

mtblanton · 2025-04-02T20:00:59Z

src/message/create_message_service.py

            yield format_message(message.MessageStreamError(reply.id, err, "grpc inference failed"))

        except multiprocessing.TimeoutError:
-            finish_reason = FinishReason.ModelOverloaded


Why was this removed?

It is changed to yielding error msg directly. I can change it back if you prefer the previous way

Doesn't this affect how we save the message at the end?

It does: https://github.com/allenai/olmo-api/pull/262/files#diff-372c652601d8e3f4422f10f598bb1bd952896faf1d09eb8f0529c4d412d5c3d0R477

Good call. Curious why only TimeoutError exception is saved as finish_reason. Should we save RpcError and ValueError in finish_reason when they happen?

ValueError may be a good finish reason, maybe something like FinishReason.InvalidRequest? I think RpcError is caused by a bad connection to InferD, so we may not need to save that?

I'll name it to BadConnection

You may want to keep that as-is or change the UI as well. We have some code tied to these values. https://github.com/allenai/olmo-ui/blob/dev/src/api/Message.ts#L99-L119

I'll raise a PR to add them

mtblanton · 2025-04-03T00:17:06Z

src/message/v4MessageBlueprint.py


-        except ValidationError as e:
-            return handle_validation_error(e)
+        except Exception as e:
+            return handle(e)



Why did this change? This was supposed to be a special case for ValidationErrors.

What would happen if the error is not ValidationError type? I saw the handle() function has more handling over different error types.

The flask error handler would handle it and return it as an exception. ValidationError handling is here to get around flask_pydantic_api's handling of ValidationErrors. I think you should revert this.

* main: increase timeout to receive first token (#263) handle the server-overloaded case (#261)

mtblanton

Nice work!

Show context length error msg

1cd282c

yensung requested a review from mtblanton April 2, 2025 19:55

mtblanton requested changes Apr 2, 2025

View reviewed changes

mtblanton reviewed Apr 3, 2025

View reviewed changes

yensung added 3 commits April 3, 2025 11:35

feedback

33c6703

Rename to bad connection error

5e6dcfd

Bad connection

04fc211

yensung requested a review from mtblanton April 3, 2025 18:39

yensung added 2 commits April 4, 2025 14:08

revert

b7b20f4

remove unused import

8c10980

yensung mentioned this pull request Apr 4, 2025

Add error types allenai/olmo-ui#943

Merged

yensung added 2 commits April 4, 2025 14:13

Add import

d9d32ad

Merge branch 'main' into 248ContextLengthError

c933298

* main: increase timeout to receive first token (#263) handle the server-overloaded case (#261)

mtblanton approved these changes Apr 5, 2025

View reviewed changes

yensung merged commit cf87e5f into main Apr 7, 2025
3 checks passed

yensung deleted the 248ContextLengthError branch April 7, 2025 16:26

mtblanton pushed a commit that referenced this pull request Jun 13, 2025

Show context length error msg (#262)

698bd8e

		except ValueError as e:
		yield format_message(message.MessageStreamError(reply.id, f"{e}", "Context length is too long"))

Conversation

yensung commented Apr 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtblanton Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yensung Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtblanton left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mtblanton Apr 2, 2025 •

edited

Loading

yensung Apr 2, 2025 •

edited

Loading