graphql-python / graphql-python/graphql-core-legacy

Usage of Asserts Breaks Code

オープン
#244 コメント 5 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
371
フォーク
175
PR マージ指標
30日以内にマージされた PR はありません

説明

This project uses asserts in a lot of places (I found about 100 cases) where they should **not** be used. They're being used to validate arguments to functions, where the normal Python way would be to use a `TypeError` or a `ValueError`. Assertions are essentially debug code, not code for validating arguments.

Any usage of optimize mode or Python (`-O` or `-OO`, or `PYTHONOPTIMIZE`) strips out assertions, making this code useless. That's very problematic if you're generating schema dynamically, as you won't get errors and this library will generate an invalid GraphQL schema.

At this point it's probably too late to change the uses of assert to a proper `TypeError` or `ValueError`, without a major version bump as dependent code in other projects may be catching `AssertionError`.

I'm proposing all instances of `assert foo, "Error"` be replaced with:

```python
if not foo:
raise AssertionError("Error") # Should be a TypeError
```

This will at least make the code work in optimize modes where assertions are stripped.

I'll gladly make a PR doing so, but it's hard to tell if this project is still actively developed.

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

調査の方向性

issueで説明されている、引数検証にassertを使用している約100箇所をリポジトリ内で検索し、通常の実行時とPython -Oでの実行時の動作を比較します。影響を受ける各呼び出し箇所について想定されるエラーメッセージを確認し、最適化された実行でも既存のAssertionError互換性を変更せずに無効な入力を引き続き拒否することを確認します。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
graphql, python
領域
backend-api-design
issue の種類
バグ
難易度
5/5
見積もり時間
1週間以上
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
25/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。