Skip to content

output(validation): Pass actual errors instead of status flag only#395

Merged
emjin merged 2 commits intomainfrom
emma/rpc-validate-errors
Aug 8, 2025
Merged

output(validation): Pass actual errors instead of status flag only#395
emjin merged 2 commits intomainfrom
emma/rpc-validate-errors

Conversation

@emjin
Copy link
Contributor

@emjin emjin commented Aug 6, 2025

Copy of #393 since it seems permissions are needed to post the comment. This will unblock the PR, and we can figure out how to fix it for next time

  • I ran make setup && make to update the generated code after editing a .atd file (TODO: have a CI check)
  • I made sure we're still backward compatible with old versions of the CLI.
    For example, the Semgrep backend need to still be able to consume data
    generated by Semgrep 1.50.0.
    See https://atd.readthedocs.io/en/latest/atdgen-tutorial.html#smooth-protocol-upgrades
    Note that the types related to the semgrep-core JSON output or the
    semgrep-core RPC do not need to be backward compatible!
  • Any accompanying changes in semgrep-proprietary are approved and ready to merge once this PR is merged

Copy of #393 since
it seems permissions are needed to post the comment. This will unblock
the PR, and we can figure out how to fix it for next time
@github-actions
Copy link

github-actions bot commented Aug 6, 2025

Backwards compatibility summary:

Checking backward compatibility of semgrep_output_v1.atd against past version v1.100.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.101.0
Skipping v1.102.0 because commit 1c82453e89e0b569630e48ddde015e201df0e5f9 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.103.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.104.0
Skipping v1.106.0 because commit 5e0c767ec323f3f2356d3bf8dbdf7c7836497d8a has already been checked
Skipping v1.107.0 because commit 5e0c767ec323f3f2356d3bf8dbdf7c7836497d8a has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.108.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.109.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.110.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.111.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.112.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.113.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.114.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.116.0
Skipping v1.117.0 because commit 5c6a8f569d16845ba10c27d17eeae68e481340d6 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.118.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.119.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.120.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.121.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.122.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.123.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.124.0
Skipping v1.124.1 because commit 75ab2f389a373af38a2a29872b4fa1c654d182f0 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.125.0
Skipping v1.126.0 because commit 02c7c65f6508daac0c9d5c0c54981731a134b038 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.127.0
Skipping v1.127.1 because commit 80fa4d2466c737b570c4f363edadc2b336e5696d has already been checked
Skipping v1.128.0 because commit 80fa4d2466c737b570c4f363edadc2b336e5696d has already been checked
Skipping v1.128.1 because commit 80fa4d2466c737b570c4f363edadc2b336e5696d has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.130.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.131.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.75.0
Skipping v1.76.0 because commit 9102031608aa4154e1c37f557550ec4eabc8780c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.77.0
Skipping v1.78.0 because commit dcb5d77b420ddee61f58aadd3c2c7aef38778154 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.79.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.80.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.81.0
Skipping v1.82.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Skipping v1.83.0 because commit 9e0f3bec26b07b4fb6753a32cb75277f45f2572c has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.84.0
Skipping v1.84.1 because commit 3daef49297ada205359cc1d2996354c94b628b0d has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.85.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.86.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.87.0
Skipping v1.88.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Skipping v1.89.0 because commit 512c0bd97db59c48a5705b2741662a338776e438 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.90.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.91.0
Skipping v1.92.0 because commit 2351c5e528cb7430422208dc66707894c066b508 has already been checked
Checking backward compatibility of semgrep_output_v1.atd against past version v1.93.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.94.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.95.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.96.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.97.0
Checking backward compatibility of semgrep_output_v1.atd against past version v1.98.0
Skipping v1.99.0 because commit 60809032a2e39742f42910d46b3e5dd305b8b8cf has already been checked

@emjin emjin requested a review from ajbt200128 August 6, 2025 06:19
ajbt200128
ajbt200128 previously approved these changes Aug 6, 2025
Copy link
Contributor

@ajbt200128 ajbt200128 left a comment

Choose a reason for hiding this comment

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

@emjin where is the matching PR for this in semgrep? Does this info get sent anywhere to the app? I'm concerned that if so we may overload the app again, like we did when we were sending timeout exceptions (which I assume is a proper subset of core_error)

Copy link
Contributor

@dijkstracula dijkstracula left a comment

Choose a reason for hiding this comment

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

Looks good - no concerns about backwards compatibility?

edit: ah, beaten to it majorly

@emjin
Copy link
Contributor Author

emjin commented Aug 6, 2025

@emjin
Copy link
Contributor Author

emjin commented Aug 6, 2025

Should be fine since this is only used internally to invoke the RPC to get rule error messages from semgrep-core, but let me think again about backwards compatibility

@dijkstracula
Copy link
Contributor

Makes sense; that was the only use of it I was aware of either, but wanted to double-check

@emjin emjin merged commit 98893da into main Aug 8, 2025
5 checks passed
@emjin emjin deleted the emma/rpc-validate-errors branch August 8, 2025 02:25
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.

3 participants