Skip to content

fix(error): propagate render error message to TUI#233

Closed
zanjonke wants to merge 1 commit into
mainfrom
fix/error-handling
Closed

fix(error): propagate render error message to TUI#233
zanjonke wants to merge 1 commit into
mainfrom
fix/error-handling

Conversation

@zanjonke

Copy link
Copy Markdown
Contributor

Summary

  • pass the render context's last error message into RenderError.get_display_message()
  • ensure the TUI receives the actual render failure message instead of the previous action payload

Testing

  • not run; PR created from existing commit

@zanjonke zanjonke requested a review from NejcS June 30, 2026 09:02
@zanjonke zanjonke self-assigned this Jun 30, 2026
@zanjonke zanjonke force-pushed the fix/error-handling branch from 286add7 to 765a6c1 Compare June 30, 2026 09:05
@zanjonke zanjonke added the bug Something isn't working label Jun 30, 2026
@NejcS

NejcS commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Your change is fixing the same problem that I fixed with the last pushed commit (6c0af01#diff-249ce6ec60216cf960c2d236774c3f21b47f4d91bdd98151833332b127b26174R36).

My two cents: I don't think we should be using the last action's payload to get the error message. So my solution was lazy (it made it work but it didn't improve the solution)

But I also think that your change is not the better option (you are now passing code_renderer.render_context.last_error_message twice into the get_display_message function.

I'll come up with a better solution in the near future.

@NejcS

NejcS commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Ah and the problem that I fixed in the referenced commit from the previous comment is that the output was something like "cannot display dict when expecting a string" in case I hit the 20 attempts limit for conformance tests. What I did is I went through all actions to find "failing" outcomes and I made sure that all instances of such outcomes are also returning RenderError.encode(message=error_msg).to_payload(), ... there were just 2 left.

@zanjonke zanjonke closed this Jun 30, 2026
@zanjonke

Copy link
Copy Markdown
Contributor Author

This was solved in #217. Thanks @NejcS .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants