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

[BUG]: DateTimeOffsetConverter doesn't parse null completed_at #595

Closed
1 task done
itrion opened this issue Oct 21, 2024 · 4 comments · Fixed by #596
Closed
1 task done

[BUG]: DateTimeOffsetConverter doesn't parse null completed_at #595

itrion opened this issue Oct 21, 2024 · 4 comments · Fixed by #596
Labels
Type: Bug Something isn't working as documented

Comments

@itrion
Copy link

itrion commented Oct 21, 2024

What happened?

Similartly to #140, the DateTimeOffsetConverter is throwing

System.Text.Json.JsonException
'Null' is not an allowed token type for the DateTimeOffsetConverter
   at Octokit.Webhooks.Converter.DateTimeOffsetConverter.ReadInternal(Utf8JsonReader& reader)
   at Octokit.Webhooks.Converter.DateTimeOffsetConverter.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
...

when parsing a workflow_job event with several "completed_at": null. The completed_at with null values are in

  • .workflow_job.completed_at
  • and .workflow_job.steps[N].completed_at

Versions

Octokit.Webhooks.AspNetCore 2.4.0

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@itrion itrion added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels Oct 21, 2024
Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@itrion itrion changed the title [BUG]: DateTimeOffsetConverter doesn't parse null created_at [BUG]: DateTimeOffsetConverter doesn't parse null completed_at Oct 21, 2024
@JamieMagee
Copy link
Contributor

JamieMagee commented Oct 21, 2024

@itrion do you have a sample payload that I can test with? It looks like workflow_job.completed_at is already nullable (though it's string? instead of DateTimeOffset?)

[JsonPropertyName("completed_at")]
public string? CompletedAt { get; init; }

And workflow_job.steps[].completed_at is already DateTimeOffset? and uses the NullableDateTimeOffsetConverter

[JsonPropertyName("completed_at")]
[JsonConverter(typeof(NullableDateTimeOffsetConverter))]
public DateTimeOffset? CompletedAt { get; init; }

@itrion
Copy link
Author

itrion commented Oct 21, 2024

Thank you @JamieMagee, after a deeper troubleshooting I realized it was actually failing parsing the started_at. Let me know if you need a full payload or if this is enough

{BA102EA2-73F3-4BBE-B129-7F6EF2668CCB}

{
	"action": "in_progress",
	"workflow_job": {
		"id": 123,
		"run_id": 1234,
		"workflow_name": "docker in  - Update",
		"head_branch": "main",
		"run_url": "https://api.github.com/repos/example/example/actions/runs/11439205892",
		"run_attempt": 1,
		"node_id": "CR_123",
		"head_sha": "96bd5bd7ac9cbc35a15cea8262abbd385778342b",
		"url": "https://api.github.com/repos/example/example/actions/jobs/31822394947",
		"html_url": "https://github.com/example/example/actions/runs/11439205892/job/31822394947",
		"status": "in_progress",
		"conclusion": null,
		"created_at": "2024-10-21T11:32:36Z",
		"started_at": "2024-10-21T11:32:47Z",
		"completed_at": null,
		"name": "Dependabot",
		"steps": [
			{
				"name": "Set up job",
				"status": "completed",
				"conclusion": "success",
				"number": 1,
				"started_at": "2024-10-21T11:32:47Z",
				"completed_at": "2024-10-21T11:32:48Z"
			},
			{
				"name": "Verify actions runner dependencies",
				"status": "in_progress",
				"conclusion": null,
				"number": 2,
				"started_at": "2024-10-21T11:32:48Z",
				"completed_at": null
			},
			{
				"name": "Create job directory",
				"status": "pending",
				"conclusion": null,
				"number": 3,
				"started_at": null,
				"completed_at": null
			},
			{
				"name": "Run Dependabot",
				"status": "pending",
				"conclusion": null,
				"number": 4,
				"started_at": null,
				"completed_at": null
			},
			{
				"name": "Cleanup job directory",
				"status": "pending",
				"conclusion": null,
				"number": 5,
				"started_at": null,
				"completed_at": null
			}
		],
		"check_run_url": "https://api.github.com/repos/example/example/check-runs/31822394947",
		"labels": [
			"dependabot"
		],
		"runner_id": 123,
		"runner_name": "dependabot",
		"runner_group_id": 25,
		"runner_group_name": "1234"
	}
}

I got to reproduce it with that payload running

	[Fact]
	public async Task ReproduceError()
	{
		var payload = await File.ReadAllTextAsync($"Fixtures/test-github-webhook-workflow_job-null-issue.json");
		var workflowJobEvent = JsonSerializer.Deserialize<WorkflowJobEvent>(payload)!;
		workflowJobEvent.Should().NotBeNull();
	}

@JamieMagee
Copy link
Contributor

Thank you! This should be resolved by #596

@kfcampbell kfcampbell moved this from 🆕 Triage to 👀 In review in 🧰 Octokit Active Oct 21, 2024
@kfcampbell kfcampbell removed the Status: Triage This is being looked at and prioritized label Oct 21, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in 🧰 Octokit Active Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants