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

Ensure we always render errors with the Append strategy #134

Merged
merged 41 commits into from
May 21, 2024

Conversation

hopsoft
Copy link
Owner

@hopsoft hopsoft commented Mar 2, 2024

Resolves #133

TLDR;

  • Better abort / error handling
  • Revamp state management
  • Add Element Attribute caching
  • Update to use Rack for all request/response modifications
  • Update to use Rails native CSRF → will likely be addressed in a separate PR for Update to use native Rails CSRF for Command POST #132

TODOs

  • Verify things work with turbo-frames with form submit
  • Add tests for turbo-frames with form submit
  • Add Abort and Error test cases
  • Ensure we properly handle Abort thrown by before_command hooks
  • Append finish event on Abort
  • Ensure we properly handle Errors raised by commands
  • Append finish even on Error
  • Make alert on Abort a config option
  • Make alert on Error a config option
  • Make raise on invalid command a config option (default: true)
  • Get window demo working with Allow behavior
  • Add debounce for state update events

Copy link

github-actions bot commented Mar 2, 2024

AppMap runtime code review

Summary Status
Failed tests ✅ All tests passed
Security flaws ✅ None detected
Performance problems ✅ None detected
Code anti-patterns 🚨 1 new
New AppMaps ⭐ 56 new minitest tests
Removed AppMaps ✖️ 8 removed minitest tests

Warnings occurred during analysis:

(apiDiff) Error comparing OpenAPI definitions: Validation errors in "head": Swagger schema validation failed. 
  Data does not match any schemas from 'oneOf' at #/paths//tests/get/responses/285
    Missing required property: description at #/paths//tests/get/responses/285
    Missing required property: $ref at #/paths//tests/get/responses/285
 
JSON_OBJECT_VALIDATION_FAILED

Code anti-patterns

🚨 New problems detected (1)

HTTP 500 status code
Description

HTTP 500 status code

Field Value
Rule http-500
Impact domain Stability
View in AppMap

⭐ New AppMaps

[minitest] Drivers window command that ALLOWS the rails controller action to perform handles abort from test/system/tests/drivers/window_test.rb:48

[minitest] Drivers window command that ALLOWS the rails controller action to perform handles error from test/system/tests/drivers/window_test.rb:59

[minitest] Turbo boost/commands/command validator valid? with inherited methods from test/turbo_boost/commands/command_validator_test.rb:111

[minitest] Turbo boost/commands/command validator valid? with invalid class from test/turbo_boost/commands/command_validator_test.rb:76

[minitest] Turbo boost/commands/command validator valid? with missing class from test/turbo_boost/commands/command_validator_test.rb:62

[minitest] Turbo boost/commands/command validator valid? with private methods from test/turbo_boost/commands/command_validator_test.rb:104

[minitest] Turbo boost/commands/command validator valid? with protected methods from test/turbo_boost/commands/command_validator_test.rb:97

[minitest] Turbo boost/commands/command validator valid? with public methods that accept arguments from test/turbo_boost/commands/command_validator_test.rb:135

[minitest] Turbo boost/commands/command validator valid? with public methods from test/turbo_boost/commands/command_validator_test.rb:90

[minitest] Turbo boost/commands/command validator validate with inherited methods from test/turbo_boost/commands/command_validator_test.rb:153

Because there are many new AppMaps, some of them are not listed in this report.

✖️ Removed AppMaps

[minitest] Turbo boost/commands/state fetch

[minitest] Turbo boost/commands/state new with expiration

[minitest] Turbo boost/commands/state new with size

[minitest] Turbo boost/commands/state read and write with expiration

[minitest] Turbo boost/commands/state read and write

[minitest] Turbo boost/commands/state state.now cannot have a now

[minitest] Turbo boost/commands/state to h

[minitest] Turbo boost/commands/state to sgid param and from sgid param

@hopsoft hopsoft marked this pull request as draft March 2, 2024 17:05
@hopsoft hopsoft marked this pull request as ready for review March 26, 2024 19:23
@hopsoft hopsoft merged commit d27d846 into main May 21, 2024
9 of 10 checks passed
@hopsoft hopsoft deleted the hopsoft/render-errors branch May 21, 2024 04:01
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.

Window driver fails to render if command raises an error
1 participant